liyafan82 commented on a change in pull request #2079:
URL: https://github.com/apache/calcite/pull/2079#discussion_r460701261
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/JoinCommuteRule.java
##########
@@ -160,28 +176,7 @@ public void onMatch(final RelOptRuleCall call) {
if (swapped == null) {
return;
}
-
- // The result is either a Project or, if the project is trivial, a
- // raw Join.
- final Join newJoin =
- swapped instanceof Join
- ? (Join) swapped
- : (Join) swapped.getInput(0);
-
call.transformTo(swapped);
-
- // We have converted join='a join b' into swapped='select
- // a0,a1,a2,b0,b1 from b join a'. Now register that project='select
- // b0,b1,a0,a1,a2 from (select a0,a1,a2,b0,b1 from b join a)' is the
- // same as 'b join a'. If we didn't do this, the swap join rule
- // would fire on the new join, ad infinitum.
- final RelBuilder relBuilder = call.builder();
- final List<RexNode> exps =
- RelOptUtil.createSwappedJoinExprs(newJoin, join, false);
- relBuilder.push(swapped)
- .project(exps, newJoin.getRowType().getFieldNames());
-
- call.getPlanner().ensureRegistered(relBuilder.build(), newJoin);
Review comment:
@rubenada Thanks a lot for your feedback.
I am sorry I failed to figure out why the removed code could prevent endless
rule firing.
To make discussion easier, I prepared a figure to illustrate what was
happening:
The transform from (1) to (2) corresponds to main part of the rule, and the
transform from (2) to (3) corresponds to the removed code. It seems Project1
and Project2 are inverse mappings, so their composite mapping amounts to an
identity mapping.
I could not figure out why this would prevent further rule firing.
----------------------------------------------------------------
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:
[email protected]
