You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2021/04/15 11:37:42 UTC

[GitHub] [calcite] zabetak commented on a change in pull request #2393: [CALCITE-4574] Wrong/Invalid plans when using RelBuilder#join with correlations

zabetak commented on a change in pull request #2393:
URL: https://github.com/apache/calcite/pull/2393#discussion_r613992188



##########
File path: core/src/main/java/org/apache/calcite/tools/RelBuilder.java
##########
@@ -2368,24 +2368,28 @@ public RelBuilder join(JoinRelType joinType, RexNode condition,
     }
     if (correlate) {
       final CorrelationId id = Iterables.getOnlyElement(variablesSet);
-      final ImmutableBitSet requiredColumns =
-          RelOptUtil.correlationColumns(id, right.rel);
       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:
-        // Correlate does not have an ON clause.
-        // For a LEFT correlate, predicate must be evaluated first.
-        // For INNER, we can defer.
+      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)));
         right = stack.pop();
         break;
-      default:
+      case INNER:
+        // For INNER, we can defer.
         postCondition = condition;
+        break;
+      default:
+        throw new IllegalArgumentException("Correlated " + joinType + " join is not supported");
       }
+      final ImmutableBitSet requiredColumns = RelOptUtil.correlationColumns(id, right.rel);

Review comment:
       Correlation has slightly different semantics than join so I am not sure if it is always safe to perform this transformation. It may be but we should tackle it in another JIRA/PR to avoid complicating this case.




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