[GitHub] [calcite] jamesstarr commented on a change in pull request #2385: [CALCITE-1045] Supporting SubQueryRemoveRule for Joins

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

jamesstarr commented on a change in pull request #2385:
URL: https://github.com/apache/calcite/pull/2385#discussion_r656351568



##########
File path: core/src/main/java/org/apache/calcite/tools/RelBuilder.java
##########
@@ -2422,18 +2422,16 @@ public RelBuilder join(JoinRelType joinType, RexNode condition,
     }
     if (correlate) {
       final CorrelationId id = Iterables.getOnlyElement(variablesSet);
-      if (!RelOptUtil.notContainsCorrelation(left.rel, id, Litmus.IGNORE)) {
-        throw new IllegalArgumentException("variable " + id
-            + " must not be used by left input to correlation");
-      }
       // Correlate does not have an ON clause.
       switch (joinType) {
       case LEFT:
       case SEMI:
       case ANTI:
         // For a LEFT/SEMI/ANTI, predicate must be evaluated first.
         stack.push(right);
-        filter(condition.accept(new Shifter(left.rel, id, right.rel)));
+        filter(
+            RelOptUtil.correlateLeftShiftRight(getRexBuilder(),
+                left.rel, id, right.rel, condition));

Review comment:
       @zabetak, I believe you are conflating limitations of the current decorrelator and being semantically equivalent.  Sort and limit nodes can be in correlated nodes and the decorrelator could decorrelate them as long as their are no correlated variables in the limit node and it children, and the sort node only requires the correlated variables not occur in its child nodes because any correlated variable that occurs in a sort node could simple be written out.
   
   Various other trivial simplification are performed through out the RelBuilder, and this does not appear to be out of character of those simplifications.  I am hard pressed to imagine any circumstance where this would cause a breaking change out side of the expectation the this call returns a correlated node, which in that case they are not programing against the Apache Calcite public api, and the onerous is on them, not calcite.   




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