utafrali commented on code in PR #4878:
URL: https://github.com/apache/calcite/pull/4878#discussion_r3068208639
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -11435,6 +11435,33 @@ private void
checkLoptOptimizeJoinRule(LoptOptimizeJoinRule rule) {
.check();
}
+ /** Test case of
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7463";>[CALCITE-7463]
+ * UnionToFilterRule incorrectly rewrites UNION with LIMIT</a>. */
+ @Test void testUnionToFilterRuleWithLimit() {
Review Comment:
The two new tests cover `UNION` (distinct) with `LIMIT`, but `UNION ALL`
with `LIMIT` is also a broken case and is not tested. The rule applies to both
set-op types, so a third test covering `UNION ALL` would close that gap:
```java
@Test void testUnionAllToFilterRuleWithLimit() {
final String sql = "(SELECT mgr, comm FROM emp LIMIT 2)\n"
+ "UNION ALL\n"
+ "(SELECT mgr, comm FROM emp LIMIT 2)\n";
sql(sql)
.withRule(CoreRules.UNION_FILTER_TO_FILTER)
.checkUnchanged();
}
```
##########
core/src/main/java/org/apache/calcite/rel/rules/SetOpToFilterRule.java:
##########
@@ -205,13 +207,35 @@ private static RelBuilder buildSetOp(RelBuilder builder,
int count, RelNode setO
// Skip non-deterministic conditions or those containing subqueries
return Pair.of(input, null);
}
- return Pair.of(filter.getInput().stripped(), filter.getCondition());
+ final RelNode source = filter.getInput().stripped();
+ if (containsSortInProjectFilterChain(source)) {
+ return Pair.of(input, null);
+ }
+ return Pair.of(source, filter.getCondition());
+ }
+ if (containsSortInProjectFilterChain(input)) {
+ return Pair.of(input, null);
}
// For non-filter inputs, use TRUE literal as default condition.
return Pair.of(input.stripped(),
input.getCluster().getRexBuilder().makeLiteral(true));
}
+ private static boolean containsSortInProjectFilterChain(RelNode input) {
Review Comment:
The method name `containsSortInProjectFilterChain` is slightly misleading
because the while loop also traverses `Filter` nodes (not just `Project`). A
name like `containsSortBelowProjectFilterChain` or simply `hasSortInChain`
would more accurately describe what it does. Minor, but method names are part
of the long-term maintenance surface.
--
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]
