kgyrtkirk commented on code in PR #15511:
URL: https://github.com/apache/druid/pull/15511#discussion_r1419519406


##########
extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/test/CalciteSelectQueryMSQTest.java:
##########
@@ -194,4 +196,41 @@ public void testArrayAggQueryOnComplexDatatypes()
       );
     }
   }
+
+  @Test(timeout = 40000)
+  public void testJoinMultipleTablesWithWhereCondition()
+  {
+    testBuilder()
+        .queryContext(
+            ImmutableMap.of(
+                QueryContexts.ENABLE_DEBUG, true,

Review Comment:
   it doesn't affect the testcase - but it makes it easier to start working on 
a testcase; I've removed it



##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java:
##########
@@ -224,11 +224,16 @@ public CalciteRulesManager(final 
Set<ExtensionCalciteRuleProvider> extensionCalc
   public List<Program> programs(final PlannerContext plannerContext)
   {
     // Program that pre-processes the tree before letting the full-on 
VolcanoPlanner loose.
+    List<RelOptRule> hepRules = new ArrayList<RelOptRule>(REDUCTION_RULES);
+    if (plannerContext.getJoinAlgorithm().requiresSubquery()) {
+      hepRules.add(CoreRules.FILTER_INTO_JOIN);

Review Comment:
   this is a very usefull rule; and would be good to run it early for every 
plan - however it could even by itself could create `ScanQuery` beneath 
`Join`-s  [like 
these](https://github.com/kgyrtkirk/druid/commit/9f8810701caedd4f1df0146e3a0a3e46dc42a195#diff-4d1d101fd375322e3a765ae6aa738283f84967d508cf8afccf6f1572af2cd1c7R1232-R1238)
 which is good; however based on the discussions in 
https://github.com/apache/druid/issues/9843 and 
https://github.com/apache/druid/pull/9773
   ...it might not be the best for Druid; 
   
   so I've decided to only enable it when also the `FANCY_JOIN` rules are 
[added](https://github.com/apache/druid/blob/c241c6980c9e5e656da252649afb9cb5d01d31a7/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java#L351)
 - as those will pill in this rule anyway.
   
   



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to