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 89b1c2e1500 branch-2.1: [fix](nereids) fix merge project contains non
foldable expression #48321 (#48366)
89b1c2e1500 is described below
commit 89b1c2e15006856bb72a4b196c21c1bc67eba4a2
Author: yujun <yu...@selectdb.com>
AuthorDate: Thu Feb 27 10:04:08 2025 +0800
branch-2.1: [fix](nereids) fix merge project contains non foldable
expression #48321 (#48366)
cherry pick from #48321
---
.../processor/post/MergeProjectPostProcessor.java | 2 +-
.../doris/nereids/processor/post/Validator.java | 7 --
.../rules/exploration/MergeProjectsCBO.java | 1 +
.../doris/nereids/rules/rewrite/MergeProjects.java | 5 +-
.../doris/nereids/trees/plans/algebra/Project.java | 10 +++
.../trees/plans/physical/PhysicalProject.java | 19 +++++
.../org/apache/doris/nereids/util/PlanUtils.java | 24 +++++++
.../java/org/apache/doris/qe/SessionVariable.java | 19 +++++
.../data/nereids_rules_p0/test_nonfoldable.out | Bin 0 -> 2819 bytes
.../nereids_rules_p0/test_nonfoldable.groovy | 77 +++++++++++++++++++++
10 files changed, 153 insertions(+), 11 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/MergeProjectPostProcessor.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/MergeProjectPostProcessor.java
index 1c465cb790d..cc4c2c5db64 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/MergeProjectPostProcessor.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/MergeProjectPostProcessor.java
@@ -35,7 +35,7 @@ public class MergeProjectPostProcessor extends
PlanPostProcessor {
public PhysicalProject visitPhysicalProject(PhysicalProject<? extends
Plan> project, CascadesContext ctx) {
Plan child = project.child();
Plan newChild = child.accept(this, ctx);
- if (newChild instanceof PhysicalProject) {
+ if (newChild instanceof PhysicalProject &&
project.canMergeProjections((PhysicalProject) newChild)) {
List<NamedExpression> projections =
project.mergeProjections((PhysicalProject) newChild);
return (PhysicalProject) project
.withProjectionsAndChild(projections, newChild.child(0))
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/Validator.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/Validator.java
index 00102b78dfe..ade8b3c913a 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/Validator.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/Validator.java
@@ -44,13 +44,6 @@ public class Validator extends PlanPostProcessor {
@Override
public Plan visitPhysicalProject(PhysicalProject<? extends Plan> project,
CascadesContext context) {
Preconditions.checkArgument(!project.getProjects().isEmpty(), "Project
list can't be empty");
-
- Plan child = project.child();
- // Forbidden project-project, we must merge project.
- if (child instanceof PhysicalProject) {
- throw new AnalysisException("Nereids must merge a project-project
plan");
- }
-
return visit(project, context);
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/MergeProjectsCBO.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/MergeProjectsCBO.java
index ff55596722e..4637425b55d 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/MergeProjectsCBO.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/MergeProjectsCBO.java
@@ -28,6 +28,7 @@ public class MergeProjectsCBO extends
OneExplorationRuleFactory {
@Override
public Rule build() {
return logicalProject(logicalProject())
+ .when(project -> project.canMergeProjections(project.child()))
.then(project -> MergeProjects.mergeProjects(project))
.toRule(RuleType.MERGE_PROJECTS);
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeProjects.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeProjects.java
index 3ea903f8565..3347bf8ced4 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeProjects.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeProjects.java
@@ -22,7 +22,6 @@ import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
-import org.apache.doris.nereids.util.ExpressionUtils;
import java.util.List;
@@ -43,11 +42,11 @@ public class MergeProjects extends OneRewriteRuleFactory {
// TODO modify ExtractAndNormalizeWindowExpression to handle nested
window functions
// here we just don't merge two projects if there is any window
function
return logicalProject(logicalProject())
- .whenNot(project ->
ExpressionUtils.containsWindowExpression(project.getProjects())
- &&
ExpressionUtils.containsWindowExpression(project.child().getProjects()))
+ .when(project -> project.canMergeProjections(project.child()))
.then(MergeProjects::mergeProjects).toRule(RuleType.MERGE_PROJECTS);
}
+ /** merge projects */
public static Plan mergeProjects(LogicalProject<?> project) {
LogicalProject<? extends Plan> childProject = (LogicalProject<?>)
project.child();
List<NamedExpression> projectExpressions =
project.mergeProjections(childProject);
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/Project.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/Project.java
index 73d4cb36448..47c4c718eae 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/Project.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/Project.java
@@ -64,6 +64,16 @@ public interface Project {
return PlanUtils.mergeProjections(childProject.getProjects(),
getProjects());
}
+ /** check can merge two projects */
+ default boolean canMergeProjections(Project childProject) {
+ if (ExpressionUtils.containsWindowExpression(getProjects())
+ &&
ExpressionUtils.containsWindowExpression(childProject.getProjects())) {
+ return false;
+ }
+
+ return PlanUtils.canReplaceWithProjections(childProject.getProjects(),
getProjects());
+ }
+
/**
* find projects, if not found the slot, then throw AnalysisException
*/
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalProject.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalProject.java
index 02769e47524..4fff87c899b 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalProject.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalProject.java
@@ -31,6 +31,7 @@ import org.apache.doris.nereids.trees.plans.PlanType;
import org.apache.doris.nereids.trees.plans.algebra.Project;
import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
import org.apache.doris.nereids.util.Utils;
+import org.apache.doris.qe.ConnectContext;
import org.apache.doris.statistics.Statistics;
import com.google.common.base.Preconditions;
@@ -40,6 +41,7 @@ import com.google.common.collect.Lists;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
+import java.util.stream.Collectors;
/**
* Physical project plan.
@@ -88,6 +90,23 @@ public class PhysicalProject<CHILD_TYPE extends Plan>
extends PhysicalUnary<CHIL
);
}
+ @Override
+ public String shapeInfo() {
+ ConnectContext context = ConnectContext.get();
+ if (context != null
+ &&
context.getSessionVariable().getDetailShapePlanNodesSet().contains(getClass().getSimpleName()))
{
+ StringBuilder builder = new StringBuilder();
+ builder.append(getClass().getSimpleName());
+ // the internal project list's order may be unstable, especial for
join tables,
+ // so sort the projects to make it stable
+
builder.append(projects.stream().map(Expression::shapeInfo).sorted()
+ .collect(Collectors.joining(", ", "[", "]")));
+ return builder.toString();
+ } else {
+ return super.shapeInfo();
+ }
+ }
+
@Override
public boolean equals(Object o) {
if (this == o) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/PlanUtils.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/PlanUtils.java
index 7cfa7b7709e..6a60290db20 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/PlanUtils.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/PlanUtils.java
@@ -42,6 +42,7 @@ import com.google.common.collect.Sets;
import java.util.Collection;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
@@ -130,6 +131,29 @@ public class PlanUtils {
return ExpressionUtils.replace(targetExpression, replaceMap);
}
+ /**
+ * replace targetExpressions with project.
+ * if the target expression contains a slot which is an alias and its
origin expression contains
+ * non-foldable expression and the slot exits multiple times, then can not
replace.
+ * for example, target expressions: [a, a + 10], child project: [ t +
random() as a ],
+ * if replace with the projects, then result expressions: [ t + random(),
t + random() + 10 ],
+ * it will calculate random two times, this is error.
+ */
+ public static boolean canReplaceWithProjections(List<NamedExpression>
childProjects,
+ List<? extends Expression> targetExpressions) {
+ Set<Slot> nonfoldableSlots =
ExpressionUtils.generateReplaceMap(childProjects).entrySet().stream()
+ .filter(entry -> entry.getValue().containsNonfoldable())
+ .map(Entry::getKey)
+ .collect(Collectors.toSet());
+ if (nonfoldableSlots.isEmpty()) {
+ return true;
+ }
+
+ Set<Slot> counterSet = Sets.newHashSet();
+ return targetExpressions.stream().noneMatch(target -> target.anyMatch(
+ e -> (e instanceof Slot) && nonfoldableSlots.contains(e) &&
!counterSet.add((Slot) e)));
+ }
+
public static Plan skipProjectFilterLimit(Plan plan) {
if (plan instanceof LogicalProject && ((LogicalProject<?>)
plan).isAllSlots()
|| plan instanceof LogicalFilter || plan instanceof
LogicalLimit) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
index 16c22caf713..0344adb648d 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java
@@ -2038,6 +2038,8 @@ public class SessionVariable implements Serializable,
Writable {
public static final String IGNORE_SHAPE_NODE = "ignore_shape_nodes";
+ public static final String DETAIL_SHAPE_NODES = "detail_shape_nodes";
+
public Set<String> getIgnoreShapePlanNodes() {
return
Arrays.stream(ignoreShapePlanNodes.split(",[\\s]*")).collect(ImmutableSet.toImmutableSet());
}
@@ -2051,6 +2053,23 @@ public class SessionVariable implements Serializable,
Writable {
"the plan node type which is ignored in 'explain shape
plan' command"})
public String ignoreShapePlanNodes = "";
+ @VariableMgr.VarAttr(name = DETAIL_SHAPE_NODES, needForward = true, setter
= "setDetailShapePlanNodes",
+ description = {"'explain shape plan' 命令中显示详细信息的PlanNode 类型",
+ "the plan node type show detail in 'explain shape plan'
command"})
+ public String detailShapePlanNodes = "";
+
+ private Set<String> detailShapePlanNodesSet = ImmutableSet.of();
+
+ public Set<String> getDetailShapePlanNodesSet() {
+ return detailShapePlanNodesSet;
+ }
+
+ public void setDetailShapePlanNodes(String detailShapePlanNodes) {
+ this.detailShapePlanNodesSet =
Arrays.stream(detailShapePlanNodes.split(",[\\s]*"))
+ .collect(ImmutableSet.toImmutableSet());
+ this.detailShapePlanNodes = detailShapePlanNodes;
+ }
+
@VariableMgr.VarAttr(name = ENABLE_DECIMAL256, needForward = true,
description = { "控制是否在计算过程中使用Decimal256类型",
"Set to true to enable Decimal256 type" })
public boolean enableDecimal256 = false;
diff --git a/regression-test/data/nereids_rules_p0/test_nonfoldable.out
b/regression-test/data/nereids_rules_p0/test_nonfoldable.out
new file mode 100644
index 00000000000..3c96406efb6
Binary files /dev/null and
b/regression-test/data/nereids_rules_p0/test_nonfoldable.out differ
diff --git a/regression-test/suites/nereids_rules_p0/test_nonfoldable.groovy
b/regression-test/suites/nereids_rules_p0/test_nonfoldable.groovy
new file mode 100644
index 00000000000..f48d93f986e
--- /dev/null
+++ b/regression-test/suites/nereids_rules_p0/test_nonfoldable.groovy
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite('test_nonfoldable') {
+ sql 'SET enable_nereids_planner=true'
+ sql 'SET runtime_filter_mode=OFF'
+ sql 'SET enable_fallback_to_original_planner=false'
+ sql "SET ignore_shape_nodes='PhysicalDistribute'"
+ sql "SET detail_shape_nodes='PhysicalProject'"
+ sql 'SET disable_nereids_rules=PRUNE_EMPTY_PARTITION'
+
+ qt_filter_through_project_1 '''
+ explain shape plan select * from (select id + 100 as a, id + 200 as b,
id + 300 as c from t1) t where a > 999 and b > 999
+ '''
+
+ qt_filter_through_project_2 '''
+ explain shape plan select * from (select id + random(1, 10) + 100 as
a, id + 200 as b, id + 300 as c from t1) t where a > 999 and b > 999
+ '''
+
+ qt_filter_through_project_3 '''
+ explain shape plan select * from (select id + random(1, 10) + 100 as
a, id + random(1, 10) + 200 as b, id + random(1, 10) + 300 as c from t1) t
where a > 999 and b > 999
+ '''
+
+ qt_filter_through_project_4 '''
+ explain shape plan select * from (select id + 100 as a, id + 200 as b,
id + 300 as c from t1) t where a + random(1, 10) > 999 and b + random(1, 10) >
999
+ '''
+
+ qt_filter_through_project_5 '''
+ explain shape plan select * from (select id + 100 as a, id + 200 as b,
id + 300 as c from t1) t where a > 999 and b > 999 limit 10
+ '''
+
+ qt_filter_through_project_6 '''
+ explain shape plan select * from (select id + random(1, 10) + 100 as
a, id + 200 as b, id + 300 as c from t1) t where a > 999 and b > 999 limit 10
+ '''
+
+ qt_filter_through_project_7 '''
+ explain shape plan select * from (select id + random(1, 10) + 100 as
a, id + random(1, 10) + 200 as b, id + random(1, 10) + 300 as c from t1) t
where a > 999 and b > 999 limit 10
+ '''
+
+ qt_filter_through_project_8 '''
+ explain shape plan select * from (select id + 100 as a, id + 200 as b,
id + 300 as c from t1) t where a + random(1, 10) > 999 and b + random(1, 10) >
999 limit 10
+ '''
+
+ qt_merge_project_1 '''
+ explain shape plan select a as b, a as c from (select id + 100 as a
from t1) t
+ '''
+
+ qt_merge_project_2 '''
+ explain shape plan select a as b, a as c from (select id + random(1,
10) as a from t1) t
+ '''
+
+ qt_merge_project_3 '''
+ explain shape plan select a as b from (select id + random(1, 10) as a
from t1) t
+ '''
+
+ qt_merge_project_4 '''
+ explain shape plan select a + 10 + a as b from (select id + random(1,
10) as a from t1) t
+ '''
+
+ qt_merge_project_5 '''
+ explain shape plan select a as b, a + 10 as c from (select id +
random(1, 10) as a from t1) t
+ '''
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
