This is an automated email from the ASF dual-hosted git repository. yiguolei pushed a commit to branch branch-2.1 in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.1 by this push: new a32a7ba5ebc branch-2.1: [fix](Nereids) deep copy for LogicalWindow is wrong #48861 (#49014) a32a7ba5ebc is described below commit a32a7ba5ebca884264e3971735080c92e37ac3ef Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> AuthorDate: Wed Mar 19 09:57:15 2025 +0800 branch-2.1: [fix](Nereids) deep copy for LogicalWindow is wrong #48861 (#49014) Cherry-picked from #48861 Co-authored-by: morrySnow <zhangwen...@selectdb.com> --- .../LogicalWindowToPhysicalWindow.java | 2 +- .../trees/copier/LogicalPlanDeepCopier.java | 11 +++-- .../trees/plans/logical/LogicalAggregate.java | 6 +++ .../trees/plans/logical/LogicalEmptyRelation.java | 12 +++-- .../trees/plans/logical/LogicalOneRowRelation.java | 4 ++ .../suites/nereids_p0/union/or_expansion.groovy | 54 ++++++++++++++++++++++ .../transform_outer_join_to_anti.groovy | 1 - 7 files changed, 80 insertions(+), 10 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalWindowToPhysicalWindow.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalWindowToPhysicalWindow.java index 020fc6754d3..8e607bee6e5 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalWindowToPhysicalWindow.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalWindowToPhysicalWindow.java @@ -68,7 +68,7 @@ public class LogicalWindowToPhysicalWindow extends OneImplementationRuleFactory public Rule build() { return RuleType.LOGICAL_WINDOW_TO_PHYSICAL_WINDOW_RULE.build( - logicalWindow().when(LogicalWindow::isChecked).then(this::implement) + logicalWindow().then(this::implement) ); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/copier/LogicalPlanDeepCopier.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/copier/LogicalPlanDeepCopier.java index 20e72b81aa7..c499c2dddb9 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/copier/LogicalPlanDeepCopier.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/copier/LogicalPlanDeepCopier.java @@ -149,7 +149,7 @@ public class LogicalPlanDeepCopier extends DefaultPlanRewriter<DeepCopierContext List<NamedExpression> outputExpressions = aggregate.getOutputExpressions().stream() .map(o -> (NamedExpression) ExpressionDeepCopier.INSTANCE.deepCopy(o, context)) .collect(ImmutableList.toImmutableList()); - return new LogicalAggregate<>(groupByExpressions, outputExpressions, child); + return aggregate.withChildGroupByAndOutput(groupByExpressions, outputExpressions, child); } @Override @@ -194,7 +194,10 @@ public class LogicalPlanDeepCopier extends DefaultPlanRewriter<DeepCopierContext List<NamedExpression> newProjects = project.getProjects().stream() .map(p -> (NamedExpression) ExpressionDeepCopier.INSTANCE.deepCopy(p, context)) .collect(ImmutableList.toImmutableList()); - return new LogicalProject<>(newProjects, child); + List<NamedExpression> newExcepts = project.getExcepts().stream() + .map(p -> (NamedExpression) ExpressionDeepCopier.INSTANCE.deepCopy(p, context)) + .collect(ImmutableList.toImmutableList()); + return new LogicalProject<>(newProjects, newExcepts, project.isDistinct(), child); } @Override @@ -353,7 +356,7 @@ public class LogicalPlanDeepCopier extends DefaultPlanRewriter<DeepCopierContext List<Slot> generatorOutput = generate.getGeneratorOutput().stream() .map(o -> (Slot) ExpressionDeepCopier.INSTANCE.deepCopy(o, context)) .collect(ImmutableList.toImmutableList()); - return new LogicalGenerate<>(generators, generatorOutput, child); + return new LogicalGenerate<>(generators, generatorOutput, generate.getExpandColumnAlias(), child); } @Override @@ -362,7 +365,7 @@ public class LogicalPlanDeepCopier extends DefaultPlanRewriter<DeepCopierContext List<NamedExpression> windowExpressions = window.getWindowExpressions().stream() .map(w -> (NamedExpression) ExpressionDeepCopier.INSTANCE.deepCopy(w, context)) .collect(ImmutableList.toImmutableList()); - return new LogicalWindow<>(windowExpressions, child); + return new LogicalWindow<>(windowExpressions, window.isChecked(), child); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalAggregate.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalAggregate.java index c4a9b8741e9..eeb97065ff0 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalAggregate.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalAggregate.java @@ -273,6 +273,12 @@ public class LogicalAggregate<CHILD_TYPE extends Plan> hasPushed, sourceRepeat, Optional.empty(), Optional.empty(), child()); } + public LogicalAggregate<Plan> withChildGroupByAndOutput(List<Expression> groupByExpressions, + List<NamedExpression> outputExpressions, Plan child) { + return new LogicalAggregate<>(groupByExpressions, outputExpressions, normalized, ordinalIsResolved, generated, + hasPushed, sourceRepeat, Optional.empty(), Optional.empty(), child); + } + public LogicalAggregate<Plan> withChildAndOutput(CHILD_TYPE child, List<NamedExpression> outputExpressionList) { return new LogicalAggregate<>(groupByExpressions, outputExpressionList, normalized, ordinalIsResolved, diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalEmptyRelation.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalEmptyRelation.java index e4ba668378c..45180db77f6 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalEmptyRelation.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalEmptyRelation.java @@ -65,10 +65,6 @@ public class LogicalEmptyRelation extends LogicalRelation return projects; } - public LogicalEmptyRelation withProjects(List<? extends NamedExpression> projects) { - return new LogicalEmptyRelation(relationId, projects); - } - @Override public Plan withGroupExpression(Optional<GroupExpression> groupExpression) { return new LogicalEmptyRelation(relationId, projects, @@ -86,6 +82,14 @@ public class LogicalEmptyRelation extends LogicalRelation throw new RuntimeException("should not call LogicalEmptyRelation's withRelationId method"); } + public LogicalEmptyRelation withProjects(List<NamedExpression> projects) { + return new LogicalEmptyRelation(relationId, projects); + } + + public LogicalEmptyRelation withRelationIdAndProjects(RelationId relationId, List<NamedExpression> projects) { + return new LogicalEmptyRelation(relationId, projects); + } + @Override public List<Slot> computeOutput() { return projects.stream() diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalOneRowRelation.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalOneRowRelation.java index 77231dd9b35..6243c0112c1 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalOneRowRelation.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalOneRowRelation.java @@ -94,6 +94,10 @@ public class LogicalOneRowRelation extends LogicalRelation implements OneRowRela throw new RuntimeException("should not call LogicalOneRowRelation's withRelationId method"); } + public LogicalOneRowRelation withRelationIdAndProjects(RelationId relationId, List<NamedExpression> projects) { + return new LogicalOneRowRelation(relationId, projects); + } + @Override public List<Slot> computeOutput() { return projects.stream() diff --git a/regression-test/suites/nereids_p0/union/or_expansion.groovy b/regression-test/suites/nereids_p0/union/or_expansion.groovy index eb335b6caeb..7554586c3ca 100644 --- a/regression-test/suites/nereids_p0/union/or_expansion.groovy +++ b/regression-test/suites/nereids_p0/union/or_expansion.groovy @@ -200,4 +200,58 @@ suite("or_expansion") { on (oe1.k0 = oe2.k0 or oe1.k1 + 1 = oe2.k1 * 2) or oe1.k2 = 1 order by oe2.k0, oe1.k0 """ + + // test all plan node to be deep copied, include + // - LogicalOneRowRelation + // - LogicalEmptyRelation + // - LogicalFilter + // - LogicalProject + // - LogicalJoin + // - LogicalRepeat + // - LogicalAggregate + // - LogicalGenerate + // - LogicalWindow + // - LogicalPartitionTopN + // - LogicalTopN + // - LogicalLimit + explain { + sql """ + WITH cte1 AS ( + SELECT 1 AS c1, max(1) OVER() AS c3 + ) + , cte2 AS ( + SELECT c1, sum(c3) AS c3 FROM cte1 GROUP BY GROUPING SETS((c1), ()) + ) + , cte3 AS ( + SELECT c1, [1, 2, 3] AS c2, c3 FROM cte2 + ) + , cte4 AS ( + SELECT c1, c2, c3, c4 FROM cte3 LATERAL VIEW EXPLODE (c2) lv AS c4 LIMIT 10 + ) + , cte5 AS ( + SELECT * FROM cte4 WHERE c4 > 2 + ) + , cte6 AS ( + SELECT 1 AS c5 FROM (SELECT 1) t WHERE 1 < 0 + ) + , cte7 AS ( + SELECT * FROM cte5 LEFT OUTER JOIN cte6 ON cte5.c1 = cte6.c5 ORDER BY c4 LIMIT 10 + ) + , cte8 AS ( + SELECT 1 AS c1, max(1) OVER() AS c3 + ) + , cte9 AS ( + SELECT * FROM (SELECT 1 AS c6, ROW_NUMBER() OVER(PARTITION BY c3 ORDER BY c1) AS c7 FROM cte8) t WHERE c7 < 3 + ) + , cte10 AS ( + SELECT * FROM cte7 LEFT OUTER JOIN cte9 ON cte7.c1 = cte9.c6 + ) + SELECT * + FROM + cte10 a + LEFT JOIN (SELECT 1 AS c1, 2 AS c2) b ON a.c1 = b.c1 OR a.c1 = b.c2 ORDER BY c1 + """ + + contains "ANTI JOIN" + } } diff --git a/regression-test/suites/nereids_syntax_p0/transform_outer_join_to_anti.groovy b/regression-test/suites/nereids_syntax_p0/transform_outer_join_to_anti.groovy index 04c92eb1305..ccbb8fd64a8 100644 --- a/regression-test/suites/nereids_syntax_p0/transform_outer_join_to_anti.groovy +++ b/regression-test/suites/nereids_syntax_p0/transform_outer_join_to_anti.groovy @@ -85,4 +85,3 @@ suite("transform_outer_join_to_anti") { contains "ANTI JOIN" } } - --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org