-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix a null related error in queries like: #11829
Fix a null related error in queries like: #11829
Conversation
6c6baf5
to
9374b7d
Compare
queryContext.setNullHandlingEnabled(true); | ||
List<Object[]> rows = executeQuery(queryContext); | ||
assertNull(rows.get(0)[0], "Column 'col1' value should be 'null' when null handling is enabled"); | ||
assertNull(rows.get(0)[1], "Column 'col2' value should be 'null' when null handling is enabled"); |
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.
Without this path, this line failed with:
java.lang.AssertionError: Column 'col2' value should be 'null' when null handling is enabled
Expected :null
Actual :-2147483648
``` select salary from mytable where salary is null order by description option(enableNullHandling=true) ``` Which didn't return null values for not ordered columns
9374b7d
to
c1cda08
Compare
Codecov Report
@@ Coverage Diff @@
## master #11829 +/- ##
=========================================
Coverage 66.48% 66.49%
Complexity 207 207
=========================================
Files 2343 2343
Lines 126418 126428 +10
Branches 19440 19445 +5
=========================================
+ Hits 84054 84064 +10
Misses 36499 36499
Partials 5865 5865
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 15 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
Good catch!
We have detected that queries like
were incorrectly executed. Specifically, the problem was that null bitmap was not applied on projected columns that were not in the order by expression in
SelectionOrderByOperator.computePartiallyOrdered
.In the previous example, for each null value in
col2
, Pinot returns the default null value for that column instead ofnull
. This patch fixes that and adds some regression tests.