This is an automated email from the ASF dual-hosted git repository.
starocean999 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 2ac6779a0fe [fix](nereids) fix merge project contains non foldable
expression (#48321)
2ac6779a0fe is described below
commit 2ac6779a0fed716a9c3d785cd354f62b43026e93
Author: yujun <yu...@selectdb.com>
AuthorDate: Wed Feb 26 15:45:16 2025 +0800
[fix](nereids) fix merge project contains non foldable expression (#48321)
When merge projections, if parent projection contain a slot and the slot
exists multiple times, and the slot is an alias in child projection and
its origin expression contains nonfoldable expression, then cann't merge
these two projections. For example:
`project(a as b, a as c) -> project(k + random() as a)`,
If merge these two projects, it will get `project(k + random() as b, k +
random() as c)`, this will calculate random() two times, then cause
error.
But if the slot only occur one time, it can still merge the two
projections. For example:
`project(a + 100 as b) -> project(k + random() as a)`, after merge them,
it will get `project(k + random() + 100 as b)`.
---
.../processor/post/MergeProjectPostProcessor.java | 2 +-
.../doris/nereids/processor/post/Validator.java | 7 --
.../rules/exploration/MergeProjectsCBO.java | 1 +
.../rules/rewrite/DeferMaterializeTopNResult.java | 2 +-
.../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 +++++++++++++++++++++
11 files changed, 154 insertions(+), 12 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 d900d207b2f..7466889fde6 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
@@ -33,7 +33,7 @@ public class MergeProjectPostProcessor extends
PlanPostProcessor {
public PhysicalProject visitPhysicalProject(PhysicalProject<? extends
Plan> project, CascadesContext ctx) {
project = (PhysicalProject<? extends Plan>) super.visit(project, ctx);
Plan child = project.child();
- if (child instanceof PhysicalProject) {
+ if (child instanceof PhysicalProject &&
project.canMergeProjections((PhysicalProject) child)) {
List<NamedExpression> projections =
project.mergeProjections((PhysicalProject) child);
return (PhysicalProject) project
.withProjectionsAndChild(projections, child.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 6fe5f0e1c3e..1240c68c1e0 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/DeferMaterializeTopNResult.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/DeferMaterializeTopNResult.java
index b606489c773..34a8dd63139 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/DeferMaterializeTopNResult.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/DeferMaterializeTopNResult.java
@@ -150,7 +150,7 @@ public class DeferMaterializeTopNResult implements
RewriteRuleFactory {
}
return true;
})
- )).then(r -> {
+ ).when(project ->
project.canMergeProjections(project.child().child()))).then(r -> {
LogicalProject<?> upperProject = r.child();
LogicalProject<LogicalFilter<LogicalOlapScan>>
bottomProject = r.child().child().child();
List<NamedExpression> projections =
upperProject.mergeProjections(bottomProject);
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 77784253440..bb6ca154136 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 60625314835..65fbf3f036e 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
@@ -75,6 +75,16 @@ public interface Project {
return projects;
}
+ /** 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 8dfb5cf55dd..7bb1b92ce2e 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
@@ -34,6 +34,7 @@ import org.apache.doris.nereids.trees.plans.algebra.Project;
import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
import org.apache.doris.nereids.util.ExpressionUtils;
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;
@@ -49,6 +50,7 @@ import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
+import java.util.stream.Collectors;
/**
* Physical project plan.
@@ -100,6 +102,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 7438e46ddc7..4d1ba233b9b 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
@@ -45,6 +45,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;
@@ -133,6 +134,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<? extends
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 48a3aefcc30..e69f3b616e8 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
@@ -2202,6 +2202,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 static final String ENABLE_SEGMENT_CACHE = "enable_segment_cache";
public Set<String> getIgnoreShapePlanNodes() {
@@ -2217,6 +2219,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
