This is an automated email from the ASF dual-hosted git repository.
morrysnow 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 3414d72e503 [fix](mtmv) Fix set disable join reorder fail when table
row count is -1 when init join order (#55681)
3414d72e503 is described below
commit 3414d72e503cceb1918aac285cb8d467203c9e41
Author: seawinde <[email protected]>
AuthorDate: Wed Sep 10 17:03:20 2025 +0800
[fix](mtmv) Fix set disable join reorder fail when table row count is -1
when init join order (#55681)
---
.../doris/nereids/rules/rewrite/InitJoinOrder.java | 7 +-
.../doris/nereids/stats/StatsCalculatorTest.java | 42 +++++++++++-
.../org/apache/doris/nereids/util/PlanChecker.java | 80 +++++++++++++++++++---
3 files changed, 114 insertions(+), 15 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InitJoinOrder.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InitJoinOrder.java
index 970dc84c787..5129f3fdfd8 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InitJoinOrder.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InitJoinOrder.java
@@ -17,6 +17,7 @@
package org.apache.doris.nereids.rules.rewrite;
+import org.apache.doris.nereids.CascadesContext;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.rules.rewrite.StatsDerive.DeriveContext;
@@ -55,12 +56,12 @@ public class InitJoinOrder extends OneRewriteRuleFactory {
return null;
}
LogicalJoin<? extends Plan, ? extends Plan> join =
(LogicalJoin<?, ?>) ctx.root;
- return swapJoinChildrenIfNeed(join);
+ return swapJoinChildrenIfNeed(join, ctx.cascadesContext);
})
.toRule(RuleType.INIT_JOIN_ORDER);
}
- private Plan swapJoinChildrenIfNeed(LogicalJoin<? extends Plan, ? extends
Plan> join) {
+ private Plan swapJoinChildrenIfNeed(LogicalJoin<? extends Plan, ? extends
Plan> join, CascadesContext context) {
if (join.getJoinType().isLeftSemiOrAntiJoin()) {
// TODO: currently, the transform rules for right semi/anti join
is not complete,
// for example LogicalJoinSemiJoinTransposeProject (tpch 22) only
works for left semi/anti join
@@ -68,7 +69,7 @@ public class InitJoinOrder extends OneRewriteRuleFactory {
return null;
}
List<CatalogRelation> scans =
join.collectToList(CatalogRelation.class::isInstance);
- Optional<String> disableReason =
StatsCalculator.disableJoinReorderIfStatsInvalid(scans, null);
+ Optional<String> disableReason =
StatsCalculator.disableJoinReorderIfStatsInvalid(scans, context);
if (!disableReason.isPresent()) {
JoinType swapType = join.getJoinType().swap();
if (swapType == null) {
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/stats/StatsCalculatorTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/stats/StatsCalculatorTest.java
index 9f77dfddb55..0590078de22 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/stats/StatsCalculatorTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/stats/StatsCalculatorTest.java
@@ -21,6 +21,7 @@ import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.OlapTable;
import org.apache.doris.catalog.PrimitiveType;
import org.apache.doris.common.Pair;
+import org.apache.doris.nereids.CascadesContext;
import org.apache.doris.nereids.memo.Group;
import org.apache.doris.nereids.memo.GroupExpression;
import org.apache.doris.nereids.properties.DataTrait;
@@ -47,18 +48,25 @@ import
org.apache.doris.nereids.trees.plans.logical.LogicalOlapScan;
import org.apache.doris.nereids.trees.plans.logical.LogicalTopN;
import org.apache.doris.nereids.trees.plans.logical.LogicalUnion;
import org.apache.doris.nereids.types.IntegerType;
+import org.apache.doris.nereids.util.LogicalPlanBuilder;
+import org.apache.doris.nereids.util.MemoTestUtils;
import org.apache.doris.nereids.util.PlanConstructor;
+import org.apache.doris.qe.SessionVariable;
import org.apache.doris.statistics.ColumnStatistic;
import org.apache.doris.statistics.ColumnStatisticBuilder;
import org.apache.doris.statistics.Statistics;
+import org.apache.doris.statistics.TableStatsMeta;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
+import mockit.Mock;
+import mockit.MockUp;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
@@ -67,10 +75,12 @@ import java.util.Map;
import java.util.Optional;
public class StatsCalculatorTest {
- private final LogicalOlapScan scan = PlanConstructor.newLogicalOlapScan(0,
"t1", 0);
+ private final LogicalOlapScan scan1 =
PlanConstructor.newLogicalOlapScan(0, "t1", 0);
+ private final LogicalOlapScan scan2 =
PlanConstructor.newLogicalOlapScan(0, "t1", 0);
+ private final LogicalOlapScan scan3 =
PlanConstructor.newLogicalOlapScan(0, "t1", 0);
private Group newFakeGroup() {
- GroupExpression groupExpression = new GroupExpression(scan);
+ GroupExpression groupExpression = new GroupExpression(scan1);
Group group = new Group(null, groupExpression,
new LogicalProperties(Collections::emptyList, () ->
DataTrait.EMPTY_TRAIT));
group.getLogicalExpressions().remove(0);
@@ -534,4 +544,32 @@ public class StatsCalculatorTest {
Statistics outputStats = calculator.computeLimit(limit, childStats);
Assertions.assertNull(outputStats.findColumnStatistics(ia).getHotValues());
}
+
+ @Test
+ public void testDisableJoinReorderIfStatsInvalid() throws IOException {
+ LogicalJoin<?, ?> join = (LogicalJoin<?, ?>) new
LogicalPlanBuilder(scan1)
+ .join(scan2, JoinType.LEFT_OUTER_JOIN, Pair.of(0, 0))
+ .join(scan3, JoinType.LEFT_OUTER_JOIN, Pair.of(0, 0))
+ .build();
+
+ // mock StatsCalculator to return -1 for getTableRowCount
+ new MockUp<OlapTable>() {
+ @Mock
+ public long getRowCountForIndex(long indexId, boolean strict) {
+ return -1;
+ }
+ };
+ new MockUp<TableStatsMeta>() {
+ @Mock
+ public long getRowCount(long indexId) {
+ return -1;
+ }
+ };
+ CascadesContext cascadesContext =
MemoTestUtils.createCascadesContext(join);
+ cascadesContext.getConnectContext().getSessionVariable()
+ .setVarOnce(SessionVariable.DISABLE_JOIN_REORDER, "false");
+
StatsCalculator.disableJoinReorderIfStatsInvalid(ImmutableList.of(scan1, scan2,
scan3), cascadesContext);
+ // because table row count is -1, so disable join reorder
+
Assertions.assertTrue(cascadesContext.getConnectContext().getSessionVariable().isDisableJoinReorder());
+ }
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/util/PlanChecker.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/util/PlanChecker.java
index 8c8da655873..2e4361000c9 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/nereids/util/PlanChecker.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/util/PlanChecker.java
@@ -18,6 +18,7 @@
package org.apache.doris.nereids.util;
import org.apache.doris.analysis.ExplainOptions;
+import org.apache.doris.common.DdlException;
import org.apache.doris.nereids.CTEContext;
import org.apache.doris.nereids.CascadesContext;
import org.apache.doris.nereids.NereidsPlanner;
@@ -68,7 +69,9 @@ import
org.apache.doris.nereids.trees.plans.visitor.CustomRewriter;
import org.apache.doris.planner.PlanFragment;
import org.apache.doris.qe.ConnectContext;
import org.apache.doris.qe.OriginStatement;
+import org.apache.doris.qe.SessionVariable;
import org.apache.doris.qe.StmtExecutor;
+import org.apache.doris.qe.VariableMgr;
import org.apache.doris.thrift.TUniqueId;
import com.google.common.collect.ImmutableList;
@@ -305,13 +308,27 @@ public class PlanChecker {
}
public PlanChecker rewrite() {
- cascadesContext.withPlanProcess(true, () -> {
- Rewriter.getWholeTreeRewriter(cascadesContext).execute();
- cascadesContext.toMemo();
- cascadesContext.getStatementContext().setNeedPreMvRewrite(
-
PreMaterializedViewRewriter.needPreRewrite(cascadesContext));
- collectTableInfoAndInitHook(cascadesContext);
- });
+ SessionVariable sessionVariable =
cascadesContext.getConnectContext().getSessionVariable();
+ try {
+ cascadesContext.withPlanProcess(true, () -> {
+ Rewriter.getWholeTreeRewriter(cascadesContext).execute();
+ cascadesContext.toMemo();
+ cascadesContext.getStatementContext().setNeedPreMvRewrite(
+
PreMaterializedViewRewriter.needPreRewrite(cascadesContext));
+ collectTableInfoAndInitHook(cascadesContext);
+ });
+ } finally {
+ // revert Session Value, because setVarOnceInSql will set some
session variable, this would influence
+ // other test cases
+ try {
+ VariableMgr.revertSessionValue(sessionVariable);
+ // origin value init
+ sessionVariable.setIsSingleSetVar(false);
+ sessionVariable.clearSessionOriginValue();
+ } catch (DdlException e) {
+ Assertions.fail("revert Session Value fail when rewrite");
+ }
+ }
return this;
}
@@ -380,7 +397,22 @@ public class PlanChecker {
LogicalPlan parsedPlan = new NereidsParser().parseSingle(sql);
LogicalPlanAdapter parsedPlanAdaptor = new
LogicalPlanAdapter(parsedPlan, statementContext);
statementContext.setParsedStatement(parsedPlanAdaptor);
- planner.plan(parsedPlanAdaptor);
+
+ SessionVariable sessionVariable = connectContext.getSessionVariable();
+ try {
+ planner.plan(parsedPlanAdaptor);
+ } finally {
+ // revert Session Value, because setVarOnceInSql will set some
session variable, this would influence
+ // other test cases
+ try {
+ VariableMgr.revertSessionValue(sessionVariable);
+ // origin value init
+ sessionVariable.setIsSingleSetVar(false);
+ sessionVariable.clearSessionOriginValue();
+ } catch (DdlException e) {
+ Assertions.fail("revert Session Value fail when explain");
+ }
+ }
return planner;
}
@@ -693,7 +725,21 @@ public class PlanChecker {
connectContext.setStatementContext(statementContext);
LogicalPlanAdapter adapter = LogicalPlanAdapter.of(parsed);
adapter.setIsExplain(new ExplainOptions(ExplainLevel.ALL_PLAN, false));
- nereidsPlanner.plan(adapter);
+ SessionVariable sessionVariable = connectContext.getSessionVariable();
+ try {
+ nereidsPlanner.plan(adapter);
+ } finally {
+ // revert Session Value, because setVarOnceInSql will set some
session variable, this would influence
+ // other test cases
+ try {
+ VariableMgr.revertSessionValue(sessionVariable);
+ // origin value init
+ sessionVariable.setIsSingleSetVar(false);
+ sessionVariable.clearSessionOriginValue();
+ } catch (DdlException e) {
+ Assertions.fail("revert Session Value fail when explain");
+ }
+ }
consumer.accept(nereidsPlanner);
return this;
}
@@ -703,7 +749,21 @@ public class PlanChecker {
StatementContext statementContext = new
StatementContext(connectContext, new OriginStatement(sql, 0));
NereidsPlanner nereidsPlanner = new NereidsPlanner(statementContext);
connectContext.setStatementContext(statementContext);
- nereidsPlanner.plan(LogicalPlanAdapter.of(parsed));
+ SessionVariable sessionVariable = connectContext.getSessionVariable();
+ try {
+ nereidsPlanner.plan(LogicalPlanAdapter.of(parsed));
+ } finally {
+ // revert Session Value, because setVarOnceInSql will set some
session variable, this would influence
+ // other test cases
+ try {
+ VariableMgr.revertSessionValue(sessionVariable);
+ // origin value init
+ sessionVariable.setIsSingleSetVar(false);
+ sessionVariable.clearSessionOriginValue();
+ } catch (DdlException e) {
+ Assertions.fail("revert Session Value fail when plan");
+ }
+ }
consumer.accept(nereidsPlanner);
return this;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
