[GitHub] [calcite] vlsi commented on a change in pull request #2365: [CALCITE-4391] Implement JoinToJoinUnifyRule in SubstitutionVisitor

[GitBox <gi...@apache.org>]

vlsi commented on a change in pull request #2365:
URL: https://github.com/apache/calcite/pull/2365#discussion_r590065715



##########
File path: core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
##########
@@ -1550,6 +1552,44 @@ private AggregateOnCalcToAggregateUnifyRule() {
     }
   }
 
+  /** A {@link SubstitutionVisitor.UnifyRule} that matches a
+   * {@link org.apache.calcite.rel.core.Join} to a
+   * {@link org.apache.calcite.rel.core.Join}, provided
+   * that they have the same child. */
+  private static class JoinToJoinUnifyRule extends AbstractUnifyRule {
+    public static final JoinToJoinUnifyRule INSTANCE = new JoinToJoinUnifyRule();
+    private JoinToJoinUnifyRule() {
+      super(any(MutableJoin.class), any(MutableJoin.class), 0);
+    }
+
+    @Override protected UnifyResult apply(UnifyRuleCall call) {
+      MutableJoin query = (MutableJoin) call.query;
+      MutableJoin target = (MutableJoin) call.target;
+
+      // same join type
+      final JoinRelType joinRelType = sameJoinType(query.joinType, target.joinType);
+      if (joinRelType == null) {
+        return null;
+      }

Review comment:
       Please move the precondition to `matches` method




----------------------------------------------------------------
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:
users@infra.apache.org