morrySnow commented on code in PR #38393:
URL: https://github.com/apache/doris/pull/38393#discussion_r1701725675


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalWindow.java:
##########
@@ -171,61 +181,162 @@ public int hashCode() {
     }
 
     /**
-     * pushPartitionLimitThroughWindow is used to push the partitionLimit 
through the window
-     * and generate the partitionTopN. If the window can not meet the 
requirement,
-     * it will return null. So when we use this function, we need check the 
null in the outside.
+     * check and get valid window function and partition limit value
      */
-    public Optional<Plan> pushPartitionLimitThroughWindow(long partitionLimit, 
boolean hasGlobalLimit) {
+    public Pair<WindowExpression, Long> 
checkAndGetValidWindowFunc(LogicalFilter<?> filter, long partitionLimit) {
         if 
(!ConnectContext.get().getSessionVariable().isEnablePartitionTopN()) {
-            return Optional.empty();
+            return null;
         }
         // We have already done such optimization rule, so just ignore it.
-        if (child(0) instanceof LogicalPartitionTopN) {
-            return Optional.empty();
+        if (child(0) instanceof LogicalPartitionTopN
+                || (child(0) instanceof LogicalFilter
+                && child(0).child(0) != null
+                && child(0).child(0) instanceof LogicalPartitionTopN)) {
+            return null;
         }
 
         // Check the window function. There are some restrictions for window 
function:
-        // 1. The number of window function should be 1.
-        // 2. The window function should be one of the 'row_number()', 
'rank()', 'dense_rank()'.
-        // 3. The window frame should be 'UNBOUNDED' to 'CURRENT'.
-        // 4. The 'PARTITION' key and 'ORDER' key can not be empty at the same 
time.
-        if (windowExpressions.size() != 1) {
-            return Optional.empty();
-        }
-        NamedExpression windowExpr = windowExpressions.get(0);
-        if (windowExpr.children().size() != 1 || !(windowExpr.child(0) 
instanceof WindowExpression)) {
-            return Optional.empty();
-        }
+        // 1. The window function should be one of the 'row_number()', 
'rank()', 'dense_rank()'.
+        // 2. The window frame should be 'UNBOUNDED' to 'CURRENT'.
+        // 3. The 'PARTITION' key and 'ORDER' key can not be empty at the same 
time.
+        WindowExpression chosenWindowFunc = null;
+        long chosenPartitionLimit = Long.MAX_VALUE;
+        long chosenRowNumberPartitionLimit = Long.MAX_VALUE;
+        boolean hasRowNumber = false;
+        for (NamedExpression windowExpr : windowExpressions) {
+            WindowExpression windowFunc = (WindowExpression) 
windowExpr.child(0);

Review Comment:
   check before cast?



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalWindow.java:
##########
@@ -171,61 +181,162 @@ public int hashCode() {
     }
 
     /**
-     * pushPartitionLimitThroughWindow is used to push the partitionLimit 
through the window
-     * and generate the partitionTopN. If the window can not meet the 
requirement,
-     * it will return null. So when we use this function, we need check the 
null in the outside.
+     * check and get valid window function and partition limit value
      */
-    public Optional<Plan> pushPartitionLimitThroughWindow(long partitionLimit, 
boolean hasGlobalLimit) {
+    public Pair<WindowExpression, Long> 
checkAndGetValidWindowFunc(LogicalFilter<?> filter, long partitionLimit) {
         if 
(!ConnectContext.get().getSessionVariable().isEnablePartitionTopN()) {
-            return Optional.empty();
+            return null;
         }
         // We have already done such optimization rule, so just ignore it.
-        if (child(0) instanceof LogicalPartitionTopN) {
-            return Optional.empty();
+        if (child(0) instanceof LogicalPartitionTopN
+                || (child(0) instanceof LogicalFilter
+                && child(0).child(0) != null
+                && child(0).child(0) instanceof LogicalPartitionTopN)) {
+            return null;
         }
 
         // Check the window function. There are some restrictions for window 
function:
-        // 1. The number of window function should be 1.
-        // 2. The window function should be one of the 'row_number()', 
'rank()', 'dense_rank()'.
-        // 3. The window frame should be 'UNBOUNDED' to 'CURRENT'.
-        // 4. The 'PARTITION' key and 'ORDER' key can not be empty at the same 
time.
-        if (windowExpressions.size() != 1) {
-            return Optional.empty();
-        }
-        NamedExpression windowExpr = windowExpressions.get(0);
-        if (windowExpr.children().size() != 1 || !(windowExpr.child(0) 
instanceof WindowExpression)) {
-            return Optional.empty();
-        }
+        // 1. The window function should be one of the 'row_number()', 
'rank()', 'dense_rank()'.
+        // 2. The window frame should be 'UNBOUNDED' to 'CURRENT'.
+        // 3. The 'PARTITION' key and 'ORDER' key can not be empty at the same 
time.
+        WindowExpression chosenWindowFunc = null;
+        long chosenPartitionLimit = Long.MAX_VALUE;
+        long chosenRowNumberPartitionLimit = Long.MAX_VALUE;
+        boolean hasRowNumber = false;
+        for (NamedExpression windowExpr : windowExpressions) {
+            WindowExpression windowFunc = (WindowExpression) 
windowExpr.child(0);
+            if (windowFunc == null || windowExpr.children().size() != 1
+                    || !(windowExpr.child(0) instanceof WindowExpression)) {
+                continue;
+            }
 
-        WindowExpression windowFunc = (WindowExpression) windowExpr.child(0);
-        // Check the window function name.
-        if (!(windowFunc.getFunction() instanceof RowNumber
-                || windowFunc.getFunction() instanceof Rank
-                || windowFunc.getFunction() instanceof DenseRank)) {
-            return Optional.empty();
-        }
+            // Check the window function name.
+            if (!(windowFunc.getFunction() instanceof RowNumber
+                    || windowFunc.getFunction() instanceof Rank
+                    || windowFunc.getFunction() instanceof DenseRank)) {
+                continue;
+            }
 
-        // Check the partition key and order key.
-        if (windowFunc.getPartitionKeys().isEmpty() && 
windowFunc.getOrderKeys().isEmpty()) {
-            return Optional.empty();
-        }
+            // Check the partition key and order key.
+            if (windowFunc.getPartitionKeys().isEmpty() && 
windowFunc.getOrderKeys().isEmpty()) {
+                continue;
+            }
 
-        // Check the window type and window frame.
-        Optional<WindowFrame> windowFrame = windowFunc.getWindowFrame();
-        if (windowFrame.isPresent()) {
-            WindowFrame frame = windowFrame.get();
-            if (!(frame.getLeftBoundary().getFrameBoundType() == 
WindowFrame.FrameBoundType.UNBOUNDED_PRECEDING
-                    && frame.getRightBoundary().getFrameBoundType() == 
WindowFrame.FrameBoundType.CURRENT_ROW)) {
-                return Optional.empty();
+            // Check the window type and window frame.
+            Optional<WindowFrame> windowFrame = windowFunc.getWindowFrame();
+            if (windowFrame.isPresent()) {
+                WindowFrame frame = windowFrame.get();
+                if (!(frame.getLeftBoundary().getFrameBoundType() == 
WindowFrame.FrameBoundType.UNBOUNDED_PRECEDING
+                        && frame.getRightBoundary().getFrameBoundType() == 
WindowFrame.FrameBoundType.CURRENT_ROW)) {
+                    continue;
+                }
+            } else {
+                continue;
             }
+
+            // Check filter conditions.
+            if (filter != null) {
+                // We currently only support simple conditions of the form 
'column </ <=/ = constant'.
+                // We will extract some related conjuncts and do some check.
+                boolean hasPartitionLimit = false;
+                long curPartitionLimit = Long.MAX_VALUE;
+                Set<Expression> conjuncts = filter.getConjuncts();
+                Set<Expression> relatedConjuncts = 
extractRelatedConjuncts(conjuncts, windowExpr.getExprId());
+                for (Expression conjunct : relatedConjuncts) {
+                    Preconditions.checkArgument(conjunct instanceof 
BinaryOperator);

Review Comment:
   add error msg? log warning log and return null instead throw exception?



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalWindow.java:
##########
@@ -171,61 +181,162 @@ public int hashCode() {
     }
 
     /**
-     * pushPartitionLimitThroughWindow is used to push the partitionLimit 
through the window
-     * and generate the partitionTopN. If the window can not meet the 
requirement,
-     * it will return null. So when we use this function, we need check the 
null in the outside.
+     * check and get valid window function and partition limit value

Review Comment:
   add comment to explain the input parameter and return value



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalWindow.java:
##########
@@ -171,61 +181,162 @@ public int hashCode() {
     }
 
     /**
-     * pushPartitionLimitThroughWindow is used to push the partitionLimit 
through the window
-     * and generate the partitionTopN. If the window can not meet the 
requirement,
-     * it will return null. So when we use this function, we need check the 
null in the outside.
+     * check and get valid window function and partition limit value
      */
-    public Optional<Plan> pushPartitionLimitThroughWindow(long partitionLimit, 
boolean hasGlobalLimit) {
+    public Pair<WindowExpression, Long> 
checkAndGetValidWindowFunc(LogicalFilter<?> filter, long partitionLimit) {
         if 
(!ConnectContext.get().getSessionVariable().isEnablePartitionTopN()) {
-            return Optional.empty();
+            return null;
         }
         // We have already done such optimization rule, so just ignore it.
-        if (child(0) instanceof LogicalPartitionTopN) {
-            return Optional.empty();
+        if (child(0) instanceof LogicalPartitionTopN
+                || (child(0) instanceof LogicalFilter
+                && child(0).child(0) != null
+                && child(0).child(0) instanceof LogicalPartitionTopN)) {
+            return null;
         }
 
         // Check the window function. There are some restrictions for window 
function:
-        // 1. The number of window function should be 1.
-        // 2. The window function should be one of the 'row_number()', 
'rank()', 'dense_rank()'.
-        // 3. The window frame should be 'UNBOUNDED' to 'CURRENT'.
-        // 4. The 'PARTITION' key and 'ORDER' key can not be empty at the same 
time.
-        if (windowExpressions.size() != 1) {
-            return Optional.empty();
-        }
-        NamedExpression windowExpr = windowExpressions.get(0);
-        if (windowExpr.children().size() != 1 || !(windowExpr.child(0) 
instanceof WindowExpression)) {
-            return Optional.empty();
-        }
+        // 1. The window function should be one of the 'row_number()', 
'rank()', 'dense_rank()'.
+        // 2. The window frame should be 'UNBOUNDED' to 'CURRENT'.
+        // 3. The 'PARTITION' key and 'ORDER' key can not be empty at the same 
time.
+        WindowExpression chosenWindowFunc = null;
+        long chosenPartitionLimit = Long.MAX_VALUE;
+        long chosenRowNumberPartitionLimit = Long.MAX_VALUE;
+        boolean hasRowNumber = false;
+        for (NamedExpression windowExpr : windowExpressions) {
+            WindowExpression windowFunc = (WindowExpression) 
windowExpr.child(0);
+            if (windowFunc == null || windowExpr.children().size() != 1
+                    || !(windowExpr.child(0) instanceof WindowExpression)) {
+                continue;
+            }
 
-        WindowExpression windowFunc = (WindowExpression) windowExpr.child(0);
-        // Check the window function name.
-        if (!(windowFunc.getFunction() instanceof RowNumber
-                || windowFunc.getFunction() instanceof Rank
-                || windowFunc.getFunction() instanceof DenseRank)) {
-            return Optional.empty();
-        }
+            // Check the window function name.
+            if (!(windowFunc.getFunction() instanceof RowNumber
+                    || windowFunc.getFunction() instanceof Rank
+                    || windowFunc.getFunction() instanceof DenseRank)) {
+                continue;
+            }
 
-        // Check the partition key and order key.
-        if (windowFunc.getPartitionKeys().isEmpty() && 
windowFunc.getOrderKeys().isEmpty()) {
-            return Optional.empty();
-        }
+            // Check the partition key and order key.
+            if (windowFunc.getPartitionKeys().isEmpty() && 
windowFunc.getOrderKeys().isEmpty()) {
+                continue;
+            }
 
-        // Check the window type and window frame.
-        Optional<WindowFrame> windowFrame = windowFunc.getWindowFrame();
-        if (windowFrame.isPresent()) {
-            WindowFrame frame = windowFrame.get();
-            if (!(frame.getLeftBoundary().getFrameBoundType() == 
WindowFrame.FrameBoundType.UNBOUNDED_PRECEDING
-                    && frame.getRightBoundary().getFrameBoundType() == 
WindowFrame.FrameBoundType.CURRENT_ROW)) {
-                return Optional.empty();
+            // Check the window type and window frame.
+            Optional<WindowFrame> windowFrame = windowFunc.getWindowFrame();
+            if (windowFrame.isPresent()) {
+                WindowFrame frame = windowFrame.get();
+                if (!(frame.getLeftBoundary().getFrameBoundType() == 
WindowFrame.FrameBoundType.UNBOUNDED_PRECEDING
+                        && frame.getRightBoundary().getFrameBoundType() == 
WindowFrame.FrameBoundType.CURRENT_ROW)) {
+                    continue;
+                }
+            } else {
+                continue;
             }
+
+            // Check filter conditions.
+            if (filter != null) {
+                // We currently only support simple conditions of the form 
'column </ <=/ = constant'.
+                // We will extract some related conjuncts and do some check.
+                boolean hasPartitionLimit = false;
+                long curPartitionLimit = Long.MAX_VALUE;
+                Set<Expression> conjuncts = filter.getConjuncts();
+                Set<Expression> relatedConjuncts = 
extractRelatedConjuncts(conjuncts, windowExpr.getExprId());
+                for (Expression conjunct : relatedConjuncts) {
+                    Preconditions.checkArgument(conjunct instanceof 
BinaryOperator);
+                    BinaryOperator op = (BinaryOperator) conjunct;
+                    Expression leftChild = op.children().get(0);
+                    Expression rightChild = op.children().get(1);
+
+                    Preconditions.checkArgument(leftChild instanceof 
SlotReference
+                            && rightChild instanceof IntegerLikeLiteral);

Review Comment:
   add error msg? log warning log and return null instead throw exception?



-- 
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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to