github-actions[bot] commented on code in PR #63379:
URL: https://github.com/apache/doris/pull/63379#discussion_r3450891995
##########
fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/EliminateOrderByKeyTest.java:
##########
@@ -187,21 +190,67 @@ void testWindowDup() {
@Test
void testWindowFd() {
+ // leading order key a repeats the partition key, the rest are
functionally dependent on a; only b remains.
PlanChecker.from(connectContext)
.analyze("select sum(a) over (partition by a order by
a,a+1,abs(a),1-a,b) from eliminate_order_by_constant_t")
.rewrite()
.printlnTree()
.matches(logicalWindow()
.when(window -> ((WindowExpression)
window.getWindowExpressions().get(0).child(0))
- .getOrderKeys().size() == 2));
+ .getOrderKeys().size() == 1));
}
@Test
void testWindowUniform() {
+ // order by b with b=100 makes b uniform, so the only order key is
redundant. sum is not a ranking
+ // function and its frame requires an ORDER BY, so one order key is
kept instead of pruning to empty.
PlanChecker.from(connectContext)
.analyze("select sum(a) over (partition by a order by b) from
eliminate_order_by_constant_t where b=100")
.rewrite()
.printlnTree()
+ .matches(logicalWindow()
+ .when(window -> ((WindowExpression)
window.getWindowExpressions().get(0).child(0))
+ .getOrderKeys().size() == 1));
+ }
+
+ @Test
+ void testWindowPartitionKey() {
+ // an order key that repeats the window's own partition key is
constant within each partition,
+ // so it is redundant and should be pruned, leaving only the remaining
order key(s).
+ PlanChecker.from(connectContext)
+ .analyze("select sum(a) over (partition by a order by a,b)
from eliminate_order_by_constant_t")
+ .rewrite()
+ .printlnTree()
+ .matches(logicalWindow()
+ .when(window -> ((WindowExpression)
window.getWindowExpressions().get(0).child(0))
+ .getOrderKeys().size() == 1));
+ }
+
+ @Test
+ void testWindowAggKeepsOrderKeyInsteadOfEmptying() {
+ // A non-ranking window function uses its frame, which requires a
non-empty ORDER BY. Even though the
+ // only order key repeats the partition key (constant within the
partition), pruning it to empty would
+ // leave the framed window without an ORDER BY, violating
WindowFunctionChecker.checkWindowFrameBeforeFunc
+ // (and, for a ROWS frame, breaking the row-sequence contract). One
order key is kept.
+ PlanChecker.from(connectContext)
+ .analyze("select sum(b) over (partition by a order by a rows
between unbounded preceding "
+ + "and current row) from
eliminate_order_by_constant_t")
+ .rewrite()
+ .printlnTree()
+ .matches(logicalWindow()
+ .when(window -> ((WindowExpression)
window.getWindowExpressions().get(0).child(0))
Review Comment:
This assertion is stale after the final `EliminateOrderByKey` change. The
rule now calls `eliminate(dataTrait, orderKeys, partitionKeyConstants)` and no
longer has the guard that kept one key when the retained list is empty. In this
test, `a` is both the partition key and the only order key, so `eliminate(...)`
drops it and `getOrderKeys()` becomes empty, while the test still expects size
`1`.
Please update or remove the guard-era expectations here and in the nearby
aggregate/uniform tests; otherwise `EliminateOrderByKeyTest` will fail once FE
UTs reach JUnit.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
