From b95a1d094271e3210543de954c6f03653bba32a0 Mon Sep 17 00:00:00 2001 From: morrySnow Date: Thu, 13 Mar 2025 12:22:04 +0800 Subject: [PATCH] [fix](Nereids) deep copy for LogicalWindow is wrong (#48861) Related PR: #21727 #14397 Problem Summary: 1. forgot to copy isChecked flag in LogicalWindow when do deep copy 2. implement LogicalWindow To PhyscialWindow should not check isChecked flag This PR: 1. check deep copy for all plan node 2. remove check isChecked in LogicalWindow To PhyscialWindow --- .../LogicalWindowToPhysicalWindow.java | 2 +- .../trees/copier/LogicalPlanDeepCopier.java | 11 ++-- .../plans/logical/LogicalEmptyRelation.java | 12 +++-- .../plans/logical/LogicalOneRowRelation.java | 4 ++ .../nereids_p0/union/or_expansion.groovy | 54 +++++++++++++++++++ .../transform_outer_join_to_anti.groovy | 1 - 6 files changed, 74 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 020fc6754d3dce..8e607bee6e5def 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 20e72b81aa7872..c499c2dddb9407 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 Plan visitLogicalAggregate(LogicalAggregate aggregate, De List 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 Plan visitLogicalProject(LogicalProject project, DeepCopi List newProjects = project.getProjects().stream() .map(p -> (NamedExpression) ExpressionDeepCopier.INSTANCE.deepCopy(p, context)) .collect(ImmutableList.toImmutableList()); - return new LogicalProject<>(newProjects, child); + List 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 Plan visitLogicalGenerate(LogicalGenerate generate, DeepC List 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 Plan visitLogicalWindow(LogicalWindow window, DeepCopierC List 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/LogicalEmptyRelation.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalEmptyRelation.java index e4ba668378c08a..45180db77f69de 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 List getProjects() { return projects; } - public LogicalEmptyRelation withProjects(List projects) { - return new LogicalEmptyRelation(relationId, projects); - } - @Override public Plan withGroupExpression(Optional groupExpression) { return new LogicalEmptyRelation(relationId, projects, @@ -86,6 +82,14 @@ public LogicalEmptyRelation withRelationId(RelationId relationId) { throw new RuntimeException("should not call LogicalEmptyRelation's withRelationId method"); } + public LogicalEmptyRelation withProjects(List projects) { + return new LogicalEmptyRelation(relationId, projects); + } + + public LogicalEmptyRelation withRelationIdAndProjects(RelationId relationId, List projects) { + return new LogicalEmptyRelation(relationId, projects); + } + @Override public List 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 7023815c7c5b99..9b78480c6a747e 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 @@ -88,6 +88,10 @@ public LogicalOneRowRelation withRelationId(RelationId relationId) { throw new RuntimeException("should not call LogicalOneRowRelation's withRelationId method"); } + public LogicalOneRowRelation withRelationIdAndProjects(RelationId relationId, List projects) { + return new LogicalOneRowRelation(relationId, projects); + } + @Override public List 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 eb335b6caebf04..7554586c3cab95 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 04c92eb1305f1a..ccbb8fd64a8f6f 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" } } -