morrySnow commented on code in PR #63379:
URL: https://github.com/apache/doris/pull/63379#discussion_r3347840522
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalWindow.java:
##########
@@ -349,9 +350,44 @@ && child(0).child(0) instanceof LogicalPartitionTopN)) {
*/
public Plan pushPartitionLimitThroughWindow(WindowExpression windowFunc,
long partitionLimit, boolean hasGlobalLimit) {
- LogicalWindow<?> window = (LogicalWindow<?>) withChildren(new
LogicalPartitionTopN<>(windowFunc, hasGlobalLimit,
- partitionLimit, child(0)));
- return window;
+ List<OrderExpression> orderKeys =
prunePartitionKeys(windowFunc.getPartitionKeys(), windowFunc.getOrderKeys());
+ LogicalPartitionTopN<Plan> partitionTopN = new
LogicalPartitionTopN<>(windowFunc.getFunction(),
+ windowFunc.getPartitionKeys(), orderKeys, hasGlobalLimit,
partitionLimit, Optional.empty(),
+ Optional.empty(), child(0));
+ // Keep the window's order keys consistent with the generated
PartitionTopN. An order key that repeats a
+ // partition key is constant inside each partition, so dropping it
changes neither the window function
+ // result nor the required order. Pruning the window here (not only on
the PartitionTopN) avoids an
+ // inconsistent state where the window keeps [partitionKey,
...orderKeys] while its PartitionTopN child
+ // only provides [...orderKeys].
+ return
withExpressionsAndChild(pruneWindowOrderKeys(windowExpressions), partitionTopN);
+ }
+
+ private static List<NamedExpression>
pruneWindowOrderKeys(List<NamedExpression> windowExpressions) {
Review Comment:
Use a separate rule to handle the orderkey pruning of the window. This way,
even if the rule for generating partitiontopn in the window is not triggered,
it can still bring benefits.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriver.java:
##########
@@ -421,7 +421,7 @@ public PhysicalProperties
visitPhysicalPartitionTopN(PhysicalPartitionTopN<? ext
Preconditions.checkState(childDistSpec instanceof
DistributionSpecHash,
"child dist spec is not hash spec");
- return new PhysicalProperties(childDistSpec, new
OrderSpec(partitionTopN.getOrderKeys()));
+ return new PhysicalProperties(childDistSpec, new
OrderSpec(partitionTopN.getOutputOrderKeys()));
Review Comment:
add comment to explain why need add OrderSpec
--
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]
