[GitHub] [calcite] julianhyde commented on a diff in pull request #3170: [CALCITE-5669] Add rule to transform correlate to join when correlation variable is unused

["julianhyde (via GitHub)" <gi...@apache.org>]

julianhyde commented on code in PR #3170:
URL: https://github.com/apache/calcite/pull/3170#discussion_r1174451513


##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -4674,6 +4675,9 @@ private static class CorrelationCollector extends RelHomogeneousShuttle {
     private final VariableUsedVisitor vuv = new VariableUsedVisitor(this);
 
     @Override public RelNode visit(RelNode other) {
+      if (other instanceof HepRelVertex) {

Review Comment:
   You might like to cherry-pick in my 5fb70e196ddafaceaf1349378de88e48f9f232f3 commit, which adds the `RelNode.stripped()`. With that commit, this `if` might not be required.



##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -3646,6 +3649,104 @@ private void checkRightEmptyAntiJoin(JoinRelType type) {
     checkEmptyJoin(relFn(relFn));
   }
 
+  @Test void testCorrelateWithoutCorrelationToEmpty() {
+    String sql = "select * from emp e where exists (select 1 from dept where empno=null)";
+    List<RelOptRule> rules =
+        Arrays.asList(CoreRules.FILTER_REDUCE_EXPRESSIONS,
+        PruneEmptyRules.PROJECT_INSTANCE,
+        PruneEmptyRules.AGGREGATE_INSTANCE,
+        PruneEmptyRules.JOIN_RIGHT_INSTANCE,
+        CoreRules.CORRELATE_TO_JOIN_RULE);
+    sql(sql)
+        .withExpand(false)
+        .withPreRule(CoreRules.FILTER_SUB_QUERY_TO_CORRELATE)
+        .withProgram(HepProgram.builder().addRuleCollection(rules).build())
+        .check();
+  }
+
+  @Test void testInnerCorrelateWithoutCorrelationToJoin() {
+    relFn(correlationPlanBuilder(JoinRelType.INNER, false))
+        .withRule(CoreRules.CORRELATE_TO_JOIN_RULE)
+        .check();
+  }
+
+  @Test void testSemiCorrelateWithoutCorrelationToJoin() {
+    relFn(correlationPlanBuilder(JoinRelType.SEMI, false))
+        .withRule(CoreRules.CORRELATE_TO_JOIN_RULE)
+        .check();
+  }
+
+  @Test void testAntiCorrelateWithoutCorrelationToJoin() {
+    relFn(correlationPlanBuilder(JoinRelType.ANTI, false))
+        .withRule(CoreRules.CORRELATE_TO_JOIN_RULE)
+        .check();
+  }
+
+  @Test void testLeftCorrelateWithoutCorrelationToJoin() {
+    relFn(correlationPlanBuilder(JoinRelType.LEFT, false))
+        .withRule(CoreRules.CORRELATE_TO_JOIN_RULE)
+        .check();
+  }
+
+  @Test void testInnerCorrelateWithCorrelationUnchanged() {
+    relFn(correlationPlanBuilder(JoinRelType.INNER, true))
+        .withRule(CoreRules.CORRELATE_TO_JOIN_RULE)
+        .checkUnchanged();
+  }
+
+  @Test void testLeftCorrelateWithCorrelationUnchanged() {
+    relFn(correlationPlanBuilder(JoinRelType.LEFT, true))
+        .withRule(CoreRules.CORRELATE_TO_JOIN_RULE)
+        .checkUnchanged();
+  }
+
+  @Test void testSemiCorrelateWithCorrelationUnchanged() {
+    relFn(correlationPlanBuilder(JoinRelType.SEMI, true))
+        .withRule(CoreRules.CORRELATE_TO_JOIN_RULE)
+        .checkUnchanged();
+  }
+
+  @Test void testAntiCorrelateWithCorrelationUnchanged() {
+    relFn(correlationPlanBuilder(JoinRelType.ANTI, true))
+        .withRule(CoreRules.CORRELATE_TO_JOIN_RULE)
+        .checkUnchanged();
+  }
+
+  /**
+   * Provides a function that will create a simple plan correlating EMP and DEPT tables with the
+   * provided join type.

Review Comment:
   please add `<p>` at start of paragraphs



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org