-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-13922][SQL] Filter rows with null attributes in vectorized parquet reader #11749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
cc @nongli |
|
Test build #53253 has finished for PR 11749 at commit
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of commenting it this way, can you put the fraction in the benchmark name?
e.g. String with Nulls Scan (95%)
|
Can you add some test cases to columnarbatchsuite that exercises this? |
af217fe to
2d1066f
Compare
|
thanks, all comments addressed! |
|
Test build #53338 has finished for PR 11749 at commit
|
| * attribute is filtered out. | ||
| */ | ||
| public final void filterNullsInColumn(int ordinal) { | ||
| assert(!nullFilteredColumns.contains(ordinal)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this assert is necessary. I think this is perfectly valid and makes this api easier to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, with the set it's not longer required. Fixed.
|
LGTM |
|
Test build #53354 has finished for PR 11749 at commit
|
…quet reader
# What changes were proposed in this pull request?
It's common for many SQL operators to not care about reading `null` values for correctness. Currently, this is achieved by performing `isNotNull` checks (for all relevant columns) on a per-row basis. Pushing these null filters in the vectorized parquet reader should bring considerable benefits (especially for cases when the underlying data doesn't contain any nulls or contains all nulls).
## How was this patch tested?
Intel(R) Core(TM) i7-4960HQ CPU 2.60GHz
String with Nulls Scan (0%): Best/Avg Time(ms) Rate(M/s) Per Row(ns) Relative
-------------------------------------------------------------------------------------------
SQL Parquet Vectorized 1229 / 1648 8.5 117.2 1.0X
PR Vectorized 833 / 846 12.6 79.4 1.5X
PR Vectorized (Null Filtering) 732 / 782 14.3 69.8 1.7X
Intel(R) Core(TM) i7-4960HQ CPU 2.60GHz
String with Nulls Scan (50%): Best/Avg Time(ms) Rate(M/s) Per Row(ns) Relative
-------------------------------------------------------------------------------------------
SQL Parquet Vectorized 995 / 1053 10.5 94.9 1.0X
PR Vectorized 732 / 772 14.3 69.8 1.4X
PR Vectorized (Null Filtering) 725 / 790 14.5 69.1 1.4X
Intel(R) Core(TM) i7-4960HQ CPU 2.60GHz
String with Nulls Scan (95%): Best/Avg Time(ms) Rate(M/s) Per Row(ns) Relative
-------------------------------------------------------------------------------------------
SQL Parquet Vectorized 326 / 333 32.2 31.1 1.0X
PR Vectorized 190 / 200 55.1 18.2 1.7X
PR Vectorized (Null Filtering) 168 / 172 62.2 16.1 1.9X
Author: Sameer Agarwal <[email protected]>
Closes apache#11749 from sameeragarwal/perf-testing.
What changes were proposed in this pull request?
It's common for many SQL operators to not care about reading
nullvalues for correctness. Currently, this is achieved by performingisNotNullchecks (for all relevant columns) on a per-row basis. Pushing these null filters in the vectorized parquet reader should bring considerable benefits (especially for cases when the underlying data doesn't contain any nulls or contains all nulls).How was this patch tested?