ccaominh commented on a change in pull request #10056:
URL: https://github.com/apache/druid/pull/10056#discussion_r443886344
##########
File path: processing/src/main/java/org/apache/druid/segment/join/Joinables.java
##########
@@ -107,8 +107,17 @@ public static boolean isPrefixedBy(final String
columnName, final String prefix)
);
for (Query joinQuery : joinQueryLevels) {
+ // The pre-analysis needs to apply to the optimized form of
filters, as this is what will be
+ // passed to HashJoinSegmentAdapter.makeCursors().
+ // The optimize() call here means that the filter optimization
will happen twice,
+ // since the query toolchests will call optimize() later.
+ // We do this for simplicity as we cannot override what query
will get run later from this context.
+ // A more complicated approach involving wrapping the query
runner after the pre-merge decoration
+ // and moving the pre-analysis might be viable.
+ // The additional overhead of the simple approach should be low,
as the additional optimize() call
+ // on each filter will only occur once per query.
preAnalysisGroup.computeJoinFilterPreAnalysisIfAbsent(
- joinQuery.getFilter() == null ? null :
joinQuery.getFilter().toFilter(),
+ joinQuery.getFilter() == null ? null :
joinQuery.getFilter().optimize().toFilter(),
Review comment:
The code coverage checker is flagging this change as uncovered (by the
druid-processing unit tests):
https://travis-ci.org/github/apache/druid/jobs/699956397#L1415
Note: The new `CalciteQueryTest` added in this PR to druid-sql does hit this
code path.
##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -11987,6 +11992,83 @@ public void
testNestedGroupByOnInlineDataSourceWithFilter(Map<String, Object> qu
);
}
+ @Test
+ @Parameters(source = QueryContextForJoinProvider.class)
+ public void testGroupByJoinAsNativeQueryWithUnoptimizedFilter(Map<String,
Object> queryContext) throws Exception
Review comment:
Intellij inspection check is flagging this line since `Exception` is not
thrown by the method body:
https://travis-ci.org/github/apache/druid/jobs/699956394#L11326
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
