github-actions[bot] commented on code in PR #63379:
URL: https://github.com/apache/doris/pull/63379#discussion_r3417828852
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/EliminateOrderByKey.java:
##########
@@ -89,7 +89,11 @@ private static Plan eliminateWindow(LogicalWindow<Plan>
window) {
for (OrderExpression orderExpression : orderExpressions) {
orderKeys.add(orderExpression.getOrderKey());
}
- List<OrderKey> retainExpression = eliminate(dataTrait, orderKeys);
+ // an order key that repeats one of the window's own partition
keys is constant within each
+ // partition, so ordering by it is redundant and can be pruned (in
addition to the data-trait
+ // based elimination below).
+ Set<Expression> partitionKeyConstants =
ImmutableSet.copyOf(windowExpression.getPartitionKeys());
+ List<OrderKey> retainExpression = eliminate(dataTrait, orderKeys,
partitionKeyConstants);
Review Comment:
This partition-key pruning is applied to every `LogicalWindow`, not only to
PartitionTopN ranking candidates. For aggregate windows with an explicit `ROWS`
frame it can remove the only ORDER BY key after
`CheckAndStandardizeWindowFunctionAndFrame` has already accepted the frame. For
example, `sum(b) over (partition by a order by a rows between unbounded
preceding and current row)` now reaches `withOrderKeys([])`: the frame still
exists, but the analyzer invariant at
`WindowFunctionChecker.checkWindowFrameBeforeFunc()` requires a frame clause to
have an ORDER BY. That also changes the row-sequence contract for a ROWS frame
before translation sends an analytic node with empty `orderByElements`. Please
restrict the partition-key pruning to frames/functions where dropping the key
is semantically safe, such as the PartitionTopN ranking/default-range path, or
keep at least one order key when the window has an explicit ROWS frame.
--
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]
