zabetak commented on a change in pull request #2385:
URL: https://github.com/apache/calcite/pull/2385#discussion_r658834610
##########
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:
I think the change is indeed valid and useful. In terms of process
though I think this commit (change in {{RelBuilder}} + tests) deserves its own
JIRA (and to facilitate things its own PR). This will give more visibility to
the change (release notes etc), and can be merged (or reverted if needed in the
future) independently of the other changes.
Can you please create a JIRA about it with a clear summary and description
of the change? Also for the existing tests I don't think you need/should modify
them so that they generate a `Correlate`; just update the plans and add extra
tests if you deem necessary.
--
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]
