This is an automated email from the ASF dual-hosted git repository.
morrysnow pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.0 by this push:
new 79acf255690 [fix](Nereids) RewriteCteChildren not work with cost based
rewritter (#26326) (#26530)
79acf255690 is described below
commit 79acf2556900f0063e88d112174c6ee9c92dde09
Author: morrySnow <[email protected]>
AuthorDate: Wed Nov 8 10:44:12 2023 +0800
[fix](Nereids) RewriteCteChildren not work with cost based rewritter
(#26326) (#26530)
we use a map to record rewrite cte children result to avoid rewrite
twice in cost based rewritter. However, we record cte outer and
inner in one map, and use null as outer result's key, use cte id as
inner result's key. This is wrong, because every anchor has an outer,
and we could only record one outer. So when we use the cache in cost
based rewritter, we get wrong outer plan from the cache. Then the error
will be thrown as below:
```
Caused by: java.lang.IllegalArgumentException: Stats for CTE: CTEId#1 not
found
at
com.google.common.base.Preconditions.checkArgument(Preconditions.java:143)
~[guava-32.1.2-jre.jar:?]
at
org.apache.doris.nereids.stats.StatsCalculator.visitLogicalCTEConsumer(StatsCalculator.java:1049)
~[classes/:?]
at
org.apache.doris.nereids.stats.StatsCalculator.visitLogicalCTEConsumer(StatsCalculator.java:147)
~[classes/:?]
at
org.apache.doris.nereids.trees.plans.logical.LogicalCTEConsumer.accept(LogicalCTEConsumer.java:111)
~[classes/:?]
at
org.apache.doris.nereids.stats.StatsCalculator.estimate(StatsCalculator.java:222)
~[classes/:?]
at
org.apache.doris.nereids.stats.StatsCalculator.estimate(StatsCalculator.java:200)
~[classes/:?]
at
org.apache.doris.nereids.jobs.cascades.DeriveStatsJob.execute(DeriveStatsJob.java:108)
~[classes/:?]
at
org.apache.doris.nereids.jobs.scheduler.SimpleJobScheduler.executeJobPool(SimpleJobScheduler.java:39)
~[classes/:?]
at
org.apache.doris.nereids.jobs.executor.Optimizer.execute(Optimizer.java:51)
~[classes/:?]
at
org.apache.doris.nereids.jobs.rewrite.CostBasedRewriteJob.getCost(CostBasedRewriteJob.java:98)
~[classes/:?]
at
org.apache.doris.nereids.jobs.rewrite.CostBasedRewriteJob.execute(CostBasedRewriteJob.java:64)
~[classes/:?]
at
org.apache.doris.nereids.jobs.executor.AbstractBatchJobExecutor.execute(AbstractBatchJobExecutor.java:119)
~[classes/:?]
at
org.apache.doris.nereids.rules.rewrite.RewriteCteChildren.visit(RewriteCteChildren.java:72)
~[classes/:?]
at
org.apache.doris.nereids.rules.rewrite.RewriteCteChildren.visit(RewriteCteChildren.java:56)
~[classes/:?]
at
org.apache.doris.nereids.trees.plans.visitor.PlanVisitor.visitLogicalSink(PlanVisitor.java:118)
~[classes/:?]
at
org.apache.doris.nereids.trees.plans.visitor.SinkVisitor.visitLogicalResultSink(SinkVisitor.java:72)
~[classes/:?]
at
org.apache.doris.nereids.trees.plans.logical.LogicalResultSink.accept(LogicalResultSink.java:58)
~[classes/:?]
at
org.apache.doris.nereids.rules.rewrite.RewriteCteChildren.visitLogicalCTEAnchor(RewriteCteChildren.java:86)
~[classes/:?]
at
org.apache.doris.nereids.rules.rewrite.RewriteCteChildren.visitLogicalCTEAnchor(RewriteCteChildren.java:56)
~[classes/:?]
at
org.apache.doris.nereids.trees.plans.logical.LogicalCTEAnchor.accept(LogicalCTEAnchor.java:60)
~[classes/:?]
at
org.apache.doris.nereids.rules.rewrite.RewriteCteChildren.visitLogicalCTEAnchor(RewriteCteChildren.java:86)
~[classes/:?]
at
org.apache.doris.nereids.rules.rewrite.RewriteCteChildren.visitLogicalCTEAnchor(RewriteCteChildren.java:56)
~[classes/:?]
at
org.apache.doris.nereids.trees.plans.logical.LogicalCTEAnchor.accept(LogicalCTEAnchor.java:60)
~[classes/:?]
at
org.apache.doris.nereids.rules.rewrite.RewriteCteChildren.rewriteRoot(RewriteCteChildren.java:67)
~[classes/:?]
at
org.apache.doris.nereids.jobs.rewrite.CustomRewriteJob.execute(CustomRewriteJob.java:58)
~[classes/:?]
at
org.apache.doris.nereids.jobs.executor.AbstractBatchJobExecutor.execute(AbstractBatchJobExecutor.java:119)
~[classes/:?]
at
org.apache.doris.nereids.NereidsPlanner.rewrite(NereidsPlanner.java:275)
~[classes/:?]
at
org.apache.doris.nereids.NereidsPlanner.plan(NereidsPlanner.java:218)
~[classes/:?]
at
org.apache.doris.nereids.NereidsPlanner.plan(NereidsPlanner.java:118)
~[classes/:?]
at
org.apache.doris.nereids.trees.plans.commands.ExplainCommand.run(ExplainCommand.java:81)
~[classes/:?]
at
org.apache.doris.qe.StmtExecutor.executeByNereids(StmtExecutor.java:550)
~[classes/:?]
```
---
.../java/org/apache/doris/nereids/StatementContext.java | 13 ++++++++++---
.../doris/nereids/jobs/rewrite/CostBasedRewriteJob.java | 2 +-
.../doris/nereids/rules/rewrite/RewriteCteChildren.java | 12 ++++++------
regression-test/suites/nereids_syntax_p0/cte.groovy | 5 +++++
4 files changed, 22 insertions(+), 10 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java
index e3987fbc45e..e5f0ca3129f 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java
@@ -81,7 +81,10 @@ public class StatementContext {
private final Map<CTEId, Set<RelationId>> cteIdToConsumerUnderProjects =
new HashMap<>();
// Used to update consumer's stats
private final Map<CTEId, List<Pair<Map<Slot, Slot>, Group>>>
cteIdToConsumerGroup = new HashMap<>();
- private final Map<CTEId, LogicalPlan> rewrittenCtePlan = new HashMap<>();
+
+ private final Map<CTEId, LogicalPlan> rewrittenCteProducer = new
HashMap<>();
+ private final Map<CTEId, LogicalPlan> rewrittenCteConsumer = new
HashMap<>();
+
private final Set<String> viewDdlSqlSet = Sets.newHashSet();
public StatementContext() {
@@ -213,8 +216,12 @@ public class StatementContext {
return cteIdToConsumerGroup;
}
- public Map<CTEId, LogicalPlan> getRewrittenCtePlan() {
- return rewrittenCtePlan;
+ public Map<CTEId, LogicalPlan> getRewrittenCteProducer() {
+ return rewrittenCteProducer;
+ }
+
+ public Map<CTEId, LogicalPlan> getRewrittenCteConsumer() {
+ return rewrittenCteConsumer;
}
public void addViewDdlSql(String ddlSql) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/CostBasedRewriteJob.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/CostBasedRewriteJob.java
index 2e5132f4ddd..2a7f0903b25 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/CostBasedRewriteJob.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/rewrite/CostBasedRewriteJob.java
@@ -89,7 +89,7 @@ public class CostBasedRewriteJob implements RewriteJob {
CascadesContext rootCtx = currentCtx.getRoot();
if (rootCtx.getRewritePlan() instanceof LogicalCTEAnchor) {
// set subtree rewrite cache
- currentCtx.getStatementContext().getRewrittenCtePlan()
+ currentCtx.getStatementContext().getRewrittenCteProducer()
.put(currentCtx.getCurrentTree().orElse(null),
(LogicalPlan) cboCtx.getRewritePlan());
// Do Whole tree rewrite
CascadesContext rootCtxCopy =
CascadesContext.newCurrentTreeContext(rootCtx);
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/RewriteCteChildren.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/RewriteCteChildren.java
index 5aa286e67f9..3318f9990d8 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/RewriteCteChildren.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/RewriteCteChildren.java
@@ -77,14 +77,14 @@ public class RewriteCteChildren extends
DefaultPlanRewriter<CascadesContext> imp
public Plan visitLogicalCTEAnchor(LogicalCTEAnchor<? extends Plan, ?
extends Plan> cteAnchor,
CascadesContext cascadesContext) {
LogicalPlan outer;
- if
(cascadesContext.getStatementContext().getRewrittenCtePlan().containsKey(null))
{
- outer =
cascadesContext.getStatementContext().getRewrittenCtePlan().get(null);
+ if
(cascadesContext.getStatementContext().getRewrittenCteConsumer().containsKey(cteAnchor.getCteId()))
{
+ outer =
cascadesContext.getStatementContext().getRewrittenCteProducer().get(cteAnchor.getCteId());
} else {
CascadesContext outerCascadesCtx =
CascadesContext.newSubtreeContext(
Optional.empty(), cascadesContext, cteAnchor.child(1),
cascadesContext.getCurrentJobContext().getRequiredProperties());
outer = (LogicalPlan) cteAnchor.child(1).accept(this,
outerCascadesCtx);
-
cascadesContext.getStatementContext().getRewrittenCtePlan().put(null, outer);
+
cascadesContext.getStatementContext().getRewrittenCteConsumer().put(cteAnchor.getCteId(),
outer);
}
boolean reserveAnchor = outer.anyMatch(p -> {
if (p instanceof LogicalCTEConsumer) {
@@ -104,8 +104,8 @@ public class RewriteCteChildren extends
DefaultPlanRewriter<CascadesContext> imp
public Plan visitLogicalCTEProducer(LogicalCTEProducer<? extends Plan>
cteProducer,
CascadesContext cascadesContext) {
LogicalPlan child;
- if
(cascadesContext.getStatementContext().getRewrittenCtePlan().containsKey(cteProducer.getCteId()))
{
- child =
cascadesContext.getStatementContext().getRewrittenCtePlan().get(cteProducer.getCteId());
+ if
(cascadesContext.getStatementContext().getRewrittenCteProducer().containsKey(cteProducer.getCteId()))
{
+ child =
cascadesContext.getStatementContext().getRewrittenCteProducer().get(cteProducer.getCteId());
} else {
child = (LogicalPlan) cteProducer.child();
child = tryToConstructFilter(cascadesContext,
cteProducer.getCteId(), child);
@@ -118,7 +118,7 @@ public class RewriteCteChildren extends
DefaultPlanRewriter<CascadesContext> imp
CascadesContext rewrittenCtx = CascadesContext.newSubtreeContext(
Optional.of(cteProducer.getCteId()), cascadesContext,
child, PhysicalProperties.ANY);
child = (LogicalPlan) child.accept(this, rewrittenCtx);
-
cascadesContext.getStatementContext().getRewrittenCtePlan().put(cteProducer.getCteId(),
child);
+
cascadesContext.getStatementContext().getRewrittenCteProducer().put(cteProducer.getCteId(),
child);
}
return cteProducer.withChildren(child);
}
diff --git a/regression-test/suites/nereids_syntax_p0/cte.groovy
b/regression-test/suites/nereids_syntax_p0/cte.groovy
index 173f3694aea..5c346f81ef9 100644
--- a/regression-test/suites/nereids_syntax_p0/cte.groovy
+++ b/regression-test/suites/nereids_syntax_p0/cte.groovy
@@ -322,5 +322,10 @@ suite("cte") {
) tab
WHERE Id IN (1, 2)
"""
+
+ // rewrite cte children should work well with cost based rewrite rule.
rely on rewrite rule: InferSetOperatorDistinct
+ sql """
+ WITH cte_0 AS ( SELECT 1 AS a ), cte_1 AS ( SELECT 1 AS a ) select *
from cte_0, cte_1 union select * from cte_0, cte_1
+ """
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
