This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new f2156159c19 branch-4.1 : [fix](join) Do not push a no-input project
item into a join whose condition refers to it- #69748 (#69814)
f2156159c19 is described below
commit f2156159c19854281fe0ea93dc52e68c4d484e29
Author: starocean999 <[email protected]>
AuthorDate: Mon Nov 2 09:23:04 2026 +0800
branch-4.1 : [fix](join) Do not push a no-input project item into a join
whose condition refers to it- #69748 (#69814)
pick https://github.com/apache/doris/pull/69748
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
---
.../doris/nereids/rules/exploration/CBOUtils.java | 63 +++++++++++
.../join/PushDownProjectThroughInnerOuterJoin.java | 20 +++-
.../join/PushDownProjectThroughSemiJoin.java | 30 +++++-
.../PushDownProjectThroughInnerOuterJoinTest.java | 22 ++++
.../join/PushDownProjectThroughSemiJoinTest.java | 21 ++++
.../distinct_split_constant_join.out | 4 +
.../distinct_split_constant_join.groovy | 118 +++++++++++++++++++++
7 files changed, 272 insertions(+), 6 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/CBOUtils.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/CBOUtils.java
index 0ac0c6ea415..58706671456 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/CBOUtils.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/CBOUtils.java
@@ -26,6 +26,8 @@ import
org.apache.doris.nereids.trees.plans.logical.LogicalJoin;
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
import org.apache.doris.nereids.util.Utils;
+import java.util.ArrayList;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -42,6 +44,67 @@ public class CBOUtils {
.collect(Collectors.toSet());
}
+ /**
+ * Collect the exprIds of the project items which read no input, e.g. the
constant materialized by constant
+ * propagation, now() or 1 + 1.
+ *
+ * <p>Such an item can not be pushed into a child of a join: the join
condition may refer to it, and pushing the
+ * item into one side while the condition keeps referring to it from the
other side makes both children of the
+ * join output the same exprId. PhysicalHashJoin#getHashConjunctsExprIds
splits the join keys by exprId only, so
+ * both keys would be taken as the keys of one side and the required hash
properties of the other side stay
+ * empty.
+ */
+ public static Set<ExprId>
collectNoInputProjectItemIds(List<NamedExpression> projectItems) {
+ Set<ExprId> noInputProjectItemIds = new HashSet<>();
+ for (NamedExpression item : projectItems) {
+ if (item.getInputSlotExprIds().isEmpty()) {
+ noInputProjectItemIds.add(item.getExprId());
+ }
+ }
+ return noInputProjectItemIds;
+ }
+
+ /** whether the join condition refers to any of the given exprIds */
+ public static boolean joinConditionRefersTo(
+ LogicalJoin<? extends Plan, ? extends Plan> join, Set<ExprId>
exprIds) {
+ if (exprIds.isEmpty()) {
+ return false;
+ }
+ for (Expression conjunct : join.getHashJoinConjuncts()) {
+ if (refersToAny(conjunct, exprIds)) {
+ return true;
+ }
+ }
+ for (Expression conjunct : join.getOtherJoinConjuncts()) {
+ if (refersToAny(conjunct, exprIds)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static boolean refersToAny(Expression expression, Set<ExprId>
exprIds) {
+ for (Slot slot : expression.getInputSlots()) {
+ if (exprIds.contains(slot.getExprId())) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * The project above a join after pushing its items down: an item which
reads no input stays in this project and
+ * keeps its expression, an item which has been pushed down is referenced
by its output slot of the join below.
+ */
+ public static List<NamedExpression> projectAboveJoin(
+ LogicalProject<?> project, Set<ExprId> noInputProjectItemIds) {
+ List<NamedExpression> projectItems = new
ArrayList<>(project.getProjects().size());
+ for (NamedExpression item : project.getProjects()) {
+ projectItems.add(noInputProjectItemIds.contains(item.getExprId())
? item : item.toSlot());
+ }
+ return projectItems;
+ }
+
public static Plan newProjectIfNeeded(Set<ExprId> requiredExprIds, Plan
plan) {
if (requiredExprIds.equals(plan.getOutputExprIdSet())) {
return plan;
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/PushDownProjectThroughInnerOuterJoin.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/PushDownProjectThroughInnerOuterJoin.java
index 905af662a2d..a328a615d51 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/PushDownProjectThroughInnerOuterJoin.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/PushDownProjectThroughInnerOuterJoin.java
@@ -92,7 +92,7 @@ public class PushDownProjectThroughInnerOuterJoin implements
ExplorationRuleFact
}
private Plan pushdownProject(LogicalProject<LogicalJoin<GroupPlan,
GroupPlan>> project) {
- LogicalJoin<GroupPlan, GroupPlan> join = project.child();
+ LogicalJoin<? extends Plan, ? extends Plan> join = project.child();
Set<ExprId> aOutputExprIdSet = join.left().getOutputExprIdSet();
Set<ExprId> bOutputExprIdSet = join.right().getOutputExprIdSet();
@@ -124,7 +124,21 @@ public class PushDownProjectThroughInnerOuterJoin
implements ExplorationRuleFact
e instanceof Slot ? childrenSlots.get((Slot) e) : e
)).map(e -> (NamedExpression) e).collect(Collectors.toList());
}
+ // A project item which reads no input, e.g. the constant materialized
by constant propagation, now() or
+ // 1 + 1, must not be pushed into either side of the join. If the join
condition refers to such an item,
+ // pushing it into one side would make both children of the join
output the same exprId, because the
+ // condition keeps referring to it from the other side. Such an item
is evaluated from nothing, so keeping
+ // it in the project above the join loses no optimization, but the
condition can not be folded either: a
+ // constant join key makes one side contribute no key at all, which
breaks the same property derivation.
+ // So the whole pushdown is rejected for this branch, the join keeps
its columns and stays valid.
+ Set<ExprId> noInputProjectItemIds =
CBOUtils.collectNoInputProjectItemIds(projects);
+ if (CBOUtils.joinConditionRefersTo(join, noInputProjectItemIds)) {
+ return null;
+ }
for (NamedExpression namedExpression : projects) {
+ if (noInputProjectItemIds.contains(namedExpression.getExprId())) {
+ continue;
+ }
Set<ExprId> usedExprIds = namedExpression.getInputSlotExprIds();
if (aOutputExprIdSet.containsAll(usedExprIds)) {
aProjects.add(namedExpression);
@@ -156,7 +170,7 @@ public class PushDownProjectThroughInnerOuterJoin
implements ExplorationRuleFact
if (!rightContains) {
Plan newJoin = join.withChildren(newLeft, join.right());
- return new
LogicalProject<>(ImmutableList.copyOf(project.getOutput()), newJoin);
+ return new LogicalProject<>(CBOUtils.projectAboveJoin(project,
noInputProjectItemIds), newJoin);
}
Builder<NamedExpression> newBProject =
ImmutableList.<NamedExpression>builder().addAll(bProjects);
@@ -167,7 +181,7 @@ public class PushDownProjectThroughInnerOuterJoin
implements ExplorationRuleFact
Plan newRight = new LogicalProject<>(newBProject.build(),
join.right());
Plan newJoin = join.withChildren(newLeft, newRight);
- return new LogicalProject<>(ImmutableList.copyOf(project.getOutput()),
newJoin);
+ return new LogicalProject<>(CBOUtils.projectAboveJoin(project,
noInputProjectItemIds), newJoin);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/PushDownProjectThroughSemiJoin.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/PushDownProjectThroughSemiJoin.java
index 11efc82fc21..e7ab2570dac 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/PushDownProjectThroughSemiJoin.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/PushDownProjectThroughSemiJoin.java
@@ -21,6 +21,7 @@ import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.rules.exploration.CBOUtils;
import org.apache.doris.nereids.rules.exploration.ExplorationRuleFactory;
+import org.apache.doris.nereids.trees.expressions.ExprId;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.plans.GroupPlan;
@@ -66,6 +67,9 @@ public class PushDownProjectThroughSemiJoin implements
ExplorationRuleFactory {
return null;
}
Plan newLeft = pushdownProject(project);
+ if (newLeft == null) {
+ return null;
+ }
return topJoin.withChildren(newLeft, topJoin.right());
}).toRule(RuleType.PUSH_DOWN_PROJECT_THROUGH_SEMI_JOIN_LEFT),
@@ -80,6 +84,9 @@ public class PushDownProjectThroughSemiJoin implements
ExplorationRuleFactory {
return null;
}
Plan newRight = pushdownProject(project);
+ if (newRight == null) {
+ return null;
+ }
return topJoin.withChildren(topJoin.left(), newRight);
}).toRule(RuleType.PUSH_DOWN_PROJECT_THROUGH_SEMI_JOIN_RIGHT)
);
@@ -99,16 +106,33 @@ public class PushDownProjectThroughSemiJoin implements
ExplorationRuleFactory {
private Plan pushdownProject(LogicalProject<LogicalJoin<GroupPlan,
GroupPlan>> project) {
LogicalJoin<GroupPlan, GroupPlan> join = project.child();
+ // A project item which reads no input, e.g. the constant materialized
by constant propagation, now() or
+ // 1 + 1, must not be pushed into the join: the join condition may
refer to such an item, and pushing it
+ // into the child while the condition keeps referring to it from the
other side makes both children of the
+ // join output the same exprId. Keeping the item above the join while
the condition refers to it is also
+ // invalid, so the pushdown is rejected for this branch.
+ Set<ExprId> noInputProjectItemIds =
CBOUtils.collectNoInputProjectItemIds(project.getProjects());
+ if (CBOUtils.joinConditionRefersTo(join, noInputProjectItemIds)) {
+ return null;
+ }
Set<Slot> conditionLeftSlots = CBOUtils.joinChildConditionSlots(join,
true);
- List<NamedExpression> newProject = new
ArrayList<>(project.getProjects());
- Set<Slot> projectUsedSlots =
project.getProjects().stream().map(NamedExpression::toSlot)
+ List<NamedExpression> newProject = new ArrayList<>();
+ for (NamedExpression expr : project.getProjects()) {
+ if (!noInputProjectItemIds.contains(expr.getExprId())) {
+ newProject.add(expr);
+ }
+ }
+ if (newProject.isEmpty()) {
+ return null;
+ }
+ Set<Slot> projectUsedSlots =
newProject.stream().map(NamedExpression::toSlot)
.collect(Collectors.toSet());
conditionLeftSlots.stream().filter(slot ->
!projectUsedSlots.contains(slot))
.forEach(newProject::add);
Plan newLeft = new LogicalProject<>(newProject, join.left());
Plan newJoin = join.withChildren(newLeft, join.right());
- return new LogicalProject<>(ImmutableList.copyOf(project.getOutput()),
newJoin);
+ return new LogicalProject<>(CBOUtils.projectAboveJoin(project,
noInputProjectItemIds), newJoin);
}
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/exploration/join/PushDownProjectThroughInnerOuterJoinTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/exploration/join/PushDownProjectThroughInnerOuterJoinTest.java
index 4c1370da1c0..e11c0c97aab 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/exploration/join/PushDownProjectThroughInnerOuterJoinTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/exploration/join/PushDownProjectThroughInnerOuterJoinTest.java
@@ -45,6 +45,28 @@ class PushDownProjectThroughInnerOuterJoinTest implements
MemoPatternMatchSuppor
private ConnectContext connectContext =
MemoTestUtils.createConnectContext();
+ @Test
+ public void
rejectPushdownWhenProjectItemReadsNoSlotAndIsUsedByJoinCondition() {
+ // project '014' AS `id`#t1.id, t1.name, t2.id, t2.name
+ // the constant reads no slot and reuses the exprId of t1.id, and the
join below refers to t1.id:
+ // pushing the project into the join would make both children of the
join output t1.id
+ List<NamedExpression> projectExprs = ImmutableList.of(
+ new Alias(scan1.getOutput().get(0).getExprId(),
Literal.of(14), scan1.getOutput().get(0).getName()),
+ scan1.getOutput().get(1),
+ scan2.getOutput().get(0),
+ scan2.getOutput().get(1)
+ );
+ LogicalPlan plan = new LogicalPlanBuilder(scan1)
+ .join(scan2, JoinType.INNER_JOIN, Pair.of(0, 0))
+ .projectExprs(projectExprs)
+ .join(scan3, JoinType.INNER_JOIN, Pair.of(1, 1))
+ .build();
+
+ PlanChecker.from(connectContext, plan)
+
.applyExploration(PushDownProjectThroughInnerOuterJoin.INSTANCE.buildRules())
+ .checkMemo(memo -> Assertions.assertEquals(1,
memo.getRoot().getLogicalExpressions().size()));
+ }
+
@Test
public void pushBothSide() {
pushBothSideHelper(JoinType.INNER_JOIN);
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/exploration/join/PushDownProjectThroughSemiJoinTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/exploration/join/PushDownProjectThroughSemiJoinTest.java
index 7f20bc3401e..030458737d6 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/exploration/join/PushDownProjectThroughSemiJoinTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/exploration/join/PushDownProjectThroughSemiJoinTest.java
@@ -33,6 +33,7 @@ import org.apache.doris.nereids.util.PlanChecker;
import org.apache.doris.nereids.util.PlanConstructor;
import com.google.common.collect.ImmutableList;
+import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import java.util.List;
@@ -42,6 +43,26 @@ class PushDownProjectThroughSemiJoinTest implements
MemoPatternMatchSupported {
private final LogicalOlapScan scan2 =
PlanConstructor.newLogicalOlapScan(1, "t2", 0);
private final LogicalOlapScan scan3 =
PlanConstructor.newLogicalOlapScan(2, "t3", 0);
+ @Test
+ public void
rejectPushdownWhenProjectItemReadsNoSlotAndIsUsedByJoinCondition() {
+ // project '014' AS `id`#t1.id, t1.name
+ // the constant reads no slot and reuses the exprId of t1.id, and the
semi join below refers to t1.id:
+ // pushing the project into the semi join would make both children of
the join output t1.id
+ List<NamedExpression> projectExprs = ImmutableList.of(
+ new Alias(scan1.getOutput().get(0).getExprId(),
Literal.of(14), scan1.getOutput().get(0).getName()),
+ scan1.getOutput().get(1)
+ );
+ LogicalPlan plan = new LogicalPlanBuilder(scan1)
+ .join(scan2, JoinType.LEFT_SEMI_JOIN, Pair.of(0, 0))
+ .projectExprs(projectExprs)
+ .join(scan3, JoinType.INNER_JOIN, Pair.of(1, 1))
+ .build();
+
+ PlanChecker.from(MemoTestUtils.createConnectContext(), plan)
+
.applyExploration(PushDownProjectThroughSemiJoin.INSTANCE.buildRules())
+ .checkMemo(memo -> Assertions.assertEquals(1,
memo.getRoot().getLogicalExpressions().size()));
+ }
+
@Test
public void pushdownProject() {
// project (t1.id + 1) as alias, t1.name
diff --git
a/regression-test/data/nereids_rules_p0/distinct_split/distinct_split_constant_join.out
b/regression-test/data/nereids_rules_p0/distinct_split/distinct_split_constant_join.out
new file mode 100644
index 00000000000..22eaec4ddd7
--- /dev/null
+++
b/regression-test/data/nereids_rules_p0/distinct_split/distinct_split_constant_join.out
@@ -0,0 +1,4 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !constant_predicate_join --
+014 C22035 shop_99 191 322 2000000 50 100
+
diff --git
a/regression-test/suites/nereids_rules_p0/distinct_split/distinct_split_constant_join.groovy
b/regression-test/suites/nereids_rules_p0/distinct_split/distinct_split_constant_join.groovy
new file mode 100644
index 00000000000..dab195a8415
--- /dev/null
+++
b/regression-test/suites/nereids_rules_p0/distinct_split/distinct_split_constant_join.groovy
@@ -0,0 +1,118 @@
+// 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("distinct_split_constant_join") {
+ sql "set enable_nereids_planner=true"
+ sql "set be_number_for_test=1"
+ // the project above the join materializes the constant of a column which
the join condition still refers to.
+ // Such an item must not be pushed into the join, otherwise both children
of the join output the same exprId
+ // and the required hash properties of one side stay empty, see
ProjectJoinReorderHelper#normalize.
+ sql "set enable_dphyp_optimizer=true"
+ // force the shuffle plan, so the required distribution of the join
children is derived instead of being
+ // satisfied by the colocate plan; that is the path where a duplicated
exprId across the join children shows up
+ sql "set disable_colocate_plan=true"
+ sql "set multi_distinct_strategy=2"
+
+ sql "drop table if exists test_split_constant_base"
+ sql """create table test_split_constant_base (
+ org_code varchar(20),
+ dc_code varchar(20),
+ date_key date,
+ people_count bigint,
+ optimum_store_count bigint
+ ) engine=olap
+ duplicate key(org_code, dc_code, date_key)
+ distributed by hash(org_code) buckets 10
+ properties("replication_num"="1")"""
+
+ sql "drop table if exists test_split_constant_sign"
+ sql """create table test_split_constant_sign (
+ org_code varchar(20),
+ dc_code varchar(20),
+ date_key date,
+ poi_code varchar(40),
+ if_optimum int,
+ sign_duration bigint
+ ) engine=olap
+ duplicate key(org_code, dc_code, date_key, poi_code)
+ distributed by hash(org_code) buckets 10
+ properties("replication_num"="1")"""
+
+ sql "drop table if exists test_split_constant_shop"
+ sql """create table test_split_constant_shop (
+ org_code varchar(20),
+ dc_code varchar(20),
+ dc_name varchar(100)
+ ) engine=olap
+ duplicate key(org_code, dc_code, dc_name)
+ distributed by hash(org_code) buckets 10
+ properties("replication_num"="1")"""
+
+ sql """insert into test_split_constant_base
+ select '014', 'C22035', date_add('2026-10-01', number % 31), number
% 7, number % 11
+ from numbers("number"="2000")"""
+ sql """insert into test_split_constant_sign
+ select '014', 'C22035', date_add('2026-10-01', number % 31),
+ concat('poi_', number % 100), number % 2, number % 100
+ from numbers("number"="20000")"""
+ sql """insert into test_split_constant_shop
+ select '014', 'C22035', concat('shop_', number)
+ from numbers("number"="100")"""
+
+ sql "analyze table test_split_constant_base with sync"
+ sql "analyze table test_split_constant_sign with sync"
+ sql "analyze table test_split_constant_shop with sync"
+
+ // the inner aggregate is split into one cte per distinct function, and
every split group by the same
+ // org_code / dc_code columns, so the outer query joins on a column whose
exprId is defined twice.
+ // `org_code in ('014')` and `dc_code in ('C22035')` make those columns
constant, so constant propagation
+ // materializes them as `'014' AS org_code` in the project above the join.
+ order_qt_constant_predicate_join """
+ select
+ t1.org_code,
+ t1.dc_code,
+ max(t2.dc_name) as dc_name,
+ sum(ifnull(t1.people_count, 0)) as people_count,
+ sum(ifnull(t1.optimum_store_count, 0)) as optimum_store_count,
+ max(ifnull(t2.sign_count, 0)) as sign_count,
+ max(ifnull(t2.sign_optimum_store_count, 0)) as
sign_optimum_store_count,
+ max(ifnull(t2.sign_store_count, 0)) as sign_store_count
+ from test_split_constant_base t1
+ inner join (
+ select
+ t1.org_code,
+ t1.dc_code,
+ max(t2.dc_name) as dc_name,
+ count(1) as sign_count,
+ count(distinct if(t1.if_optimum = 1, t1.poi_code, null)) as
sign_optimum_store_count,
+ count(distinct t1.poi_code) as sign_store_count
+ from test_split_constant_sign t1
+ left join (
+ select distinct org_code, dc_code, dc_name from
test_split_constant_shop
+ ) t2 on t1.org_code = t2.org_code and t1.dc_code = t2.dc_code
+ where t1.date_key >= '2026-10-01' and t1.date_key <= '2026-10-31'
+ and t1.org_code in ('014')
+ and t1.dc_code in ('C22035')
+ group by t1.org_code, t1.dc_code
+ ) t2 on t1.org_code = t2.org_code and t1.dc_code = t2.dc_code
+ where t1.date_key = '2026-10-24'
+ and t1.org_code in ('014')
+ and t1.dc_code in ('C22035')
+ group by t1.org_code, t1.dc_code
+ order by 1, 2
+ """
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
