xiedeyantu commented on code in PR #4338:
URL: https://github.com/apache/calcite/pull/4338#discussion_r2062388459
##########
core/src/main/java/org/apache/calcite/rel/rules/SortJoinTransposeRule.java:
##########
@@ -116,13 +121,26 @@ public SortJoinTransposeRule(Class<? extends Sort>
sortClass,
mq, join.getLeft(), joinInfo.leftSet())) {
return false;
}
+ if (alreadyPushedDown(join.getRight())) {
+ return false;
+ }
} else {
return false;
}
return true;
}
+ // Returns true if the sort has already been pushed down
+ private boolean alreadyPushedDown(RelNode rel) {
Review Comment:
Thank you for submitting the PR. I have two small questions. The first is
whether the `sort` containing the `RexDynamicParam` parameter should be pushed
down? The second is if it can be pushed down, then in this method you only
check whether the `sort` contains `RexDynamicParam`, but can you guarantee that
this `sort` is the same as the top-level `sort`?
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -7962,6 +7963,50 @@ private void checkSemiJoinRuleOnAntiJoin(RelOptRule
rule) {
.checkUnchanged();
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-6983";>[CALCITE-6983]
+ * SortJoinTransposeRule should not push SORT past a UNION when SORT's fetch
is DynamicParam
+ </a>. */
+ @Test void testSortJoinTranspose8() {
Review Comment:
This case seems to have nothing to do with PR.
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -7962,6 +7963,50 @@ private void checkSemiJoinRuleOnAntiJoin(RelOptRule
rule) {
.checkUnchanged();
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-6983";>[CALCITE-6983]
+ * SortJoinTransposeRule should not push SORT past a UNION when SORT's fetch
is DynamicParam
+ </a>. */
+ @Test void testSortJoinTranspose8() {
+ HepProgramBuilder builder = new HepProgramBuilder();
+ builder.addRuleClass(SortProjectTransposeRule.class);
+ builder.addRuleClass(SortJoinTransposeRule.class);
+ builder.addRuleClass(SortUnionTransposeRule.class);
+ HepPlanner hepPlanner = new HepPlanner(builder.build());
+ hepPlanner.addRule(CoreRules.SORT_PROJECT_TRANSPOSE);
+ hepPlanner.addRule(CoreRules.SORT_JOIN_TRANSPOSE);
+ hepPlanner.addRule(CoreRules.SORT_UNION_TRANSPOSE);
+ final String sql = "SELECT x.empno1, y.empno2\n"
+ + " FROM (\n"
+ + " SELECT empno as empno1 from emp where empno = ?\n"
+ + " UNION ALL\n"
+ + " SELECT empno as empno1 from emp where job = ?\n"
+ + " ) AS x\n"
+ + " left join (\n"
+ + " SELECT empno as empno2 from emp where mgr = ?\n"
+ + " ) AS y on x.empno1 = y.empno2\n"
+ + " FETCH NEXT ? ROWS ONLY";
+ sql(sql).withPlanner(hepPlanner).check();
+ }
+
+ @Test void testSortJoinTranspose9() {
Review Comment:
There is a missing annotation like "/** Test case for ...".
--
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]
