clintropolis commented on a change in pull request #9251: fix issue with long
column predicate matcher based filters and nulls
URL: https://github.com/apache/druid/pull/9251#discussion_r371168263
##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -3198,6 +3198,69 @@ public void testNullLongTopN() throws Exception
);
}
+ @Test
+ public void testLongPredicateFilterNulls() throws Exception
+ {
+ testQuery(
+ "SELECT COUNT(*)\n"
+ + "FROM druid.numfoo\n"
+ + "WHERE l1 > 3",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE3)
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .granularity(Granularities.ALL)
+ .filters(bound("l1", "3", null, true, false, null,
StringComparators.NUMERIC))
Review comment:
I did some refactoring around `BaseFilterTest` to add in numeric columns
with null values, and bumped into some additional issues. First one in the
`JavascriptDimFilter`, whose numeric predicates did not implement `applyNull`,
meaning you couldn't specifically filter for the null value. The 'in' filter I
believe also has this problem, assuming that something like `.. x IN (1, 2,
NULL)` is valid, but is more involved to fix so I didn't do it in this PR
(assuming it needs fixed?).
I also ran into some issues with vectorized value and predicate matchers on
numeric null columns. The initial issue is that the matchers were not checking
the null bitmap, however once that was in place I bumped into another issue
that the null vector could be incorrectly polluted with the wrong null
information in the case where the null bitmap ran out of values before the end
of the column (likely) because it was breaking from the loop instead of writing
false values until the end vector offset.
I added tests for all of these cases.
I did not dig into the expression filter for fear of what I might run into,
but will investigate this as a follow-up to this PR, along with any remaining
filters which I did not address.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
