zabetak commented on a change in pull request #2385:
URL: https://github.com/apache/calcite/pull/2385#discussion_r679005833
##########
File path:
core/src/main/java/org/apache/calcite/rel/rules/SubQueryRemoveRule.java
##########
@@ -638,23 +639,44 @@ private static void matchFilter(SubQueryRemoveRule rule,
private static void matchJoin(SubQueryRemoveRule rule, RelOptRuleCall call) {
final Join join = call.rel(0);
- final RelBuilder builder = call.builder();
- final RexSubQuery e =
- RexUtil.SubQueryFinder.find(join.getCondition());
- assert e != null;
- final RelOptUtil.Logic logic =
- LogicVisitor.find(RelOptUtil.Logic.TRUE,
- ImmutableList.of(join.getCondition()), e);
- builder.push(join.getLeft());
- builder.push(join.getRight());
- final int fieldCount = join.getRowType().getFieldCount();
- final Set<CorrelationId> variablesSet =
- RelOptUtil.getVariablesUsed(e.rel);
- final RexNode target = rule.apply(e, variablesSet,
- logic, builder, 2, fieldCount);
- final RexShuttle shuttle = new ReplaceSubQueryShuttle(e, target);
- builder.join(join.getJoinType(), shuttle.apply(join.getCondition()));
- builder.project(fields(builder, join.getRowType().getFieldCount()));
+ switch (join.getJoinType()) {
+ case INNER:
+ case LEFT:
+ case ANTI:
+ case SEMI:
+ break;
+ default:
+ //Incorrect queries will be produced for full and right joins.
+ return;
+ }
Review comment:
Since we cannot treat `RIGHT` and `FULL` outer joins I think it makes
sense to skip calling `onMatch` method altogether. Is it possible to move the
check as rule predicate or apply the check as part of `RelOptRule#matches`?
##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
##########
@@ -305,6 +305,25 @@ public static boolean notContainsCorrelation(RelNode r,
}
}
+ /** Shifts a predicate's inputs to the left, replacing early
+ * ones with references to a {@link RexCorrelVariable}. */
+ public static RexNode correlateLeftShiftRight(RexBuilder rexBuilder,
Review comment:
It is not clear what the method does from the javadoc or method name. I
know that you moved existing code but given that now the method is public it
would be nice if we could improve the doc. You can possibly include sub-plan
examples in the javadoc to aid in the understanding of the method.
##########
File path:
core/src/main/java/org/apache/calcite/rel/rules/SubQueryRemoveRule.java
##########
@@ -638,23 +639,44 @@ private static void matchFilter(SubQueryRemoveRule rule,
private static void matchJoin(SubQueryRemoveRule rule, RelOptRuleCall call) {
final Join join = call.rel(0);
- final RelBuilder builder = call.builder();
- final RexSubQuery e =
- RexUtil.SubQueryFinder.find(join.getCondition());
- assert e != null;
- final RelOptUtil.Logic logic =
- LogicVisitor.find(RelOptUtil.Logic.TRUE,
- ImmutableList.of(join.getCondition()), e);
- builder.push(join.getLeft());
- builder.push(join.getRight());
- final int fieldCount = join.getRowType().getFieldCount();
- final Set<CorrelationId> variablesSet =
- RelOptUtil.getVariablesUsed(e.rel);
- final RexNode target = rule.apply(e, variablesSet,
- logic, builder, 2, fieldCount);
- final RexShuttle shuttle = new ReplaceSubQueryShuttle(e, target);
- builder.join(join.getJoinType(), shuttle.apply(join.getCondition()));
- builder.project(fields(builder, join.getRowType().getFieldCount()));
+ switch (join.getJoinType()) {
+ case INNER:
+ case LEFT:
+ case ANTI:
+ case SEMI:
+ break;
+ default:
+ //Incorrect queries will be produced for full and right joins.
+ return;
+ }
+ final RelBuilder builder = call.builder()
+ .push(join.getLeft())
+ .push(join.getRight());
+ final CorrelationId id = join.getCluster().createCorrel();
+ RexNode condition =
RelOptUtil.correlateLeftShiftRight(builder.getRexBuilder(),
+ join.getLeft(), id, join.getRight(), join.getCondition());
+ boolean found = false;
+ while (true) {
Review comment:
I've seen that `matchFilter` shares some similarities with this newly
added code. Is it correct to assume that we need to do this also for
`matchProject`? Should we log a follow-up JIRA?
--
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]
