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