This is an automated email from the ASF dual-hosted git repository.
englefly 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 02cb614b1ed [fix](rbo) Give the constants materialized by constant
propagation a fresh exprId (#69111)
02cb614b1ed is described below
commit 02cb614b1ed5e11f21cd782a8af5b9432820a4e8
Author: minghong <[email protected]>
AuthorDate: Wed Oct 21 19:05:09 2026 +0800
[fix](rbo) Give the constants materialized by constant propagation a fresh
exprId (#69111)
### What problem does this PR solve?
Problem Summary:
When ConstantPropagation replaces an aggregate output by a constant, it
materializes the constant in a
project above the aggregate, and the project item reused the exprId of
the replaced output:
LogicalProject ( projects=[..., 1 AS `id`#1, ...] )
+-- LogicalAggregate ( groupByExpr=[id#1], outputExpr=[id#1] )
So one exprId had two definitions, the aggregate output slot and the
constant, whose nullable and data
type may differ. Rules which compare exprIds only then take them as the
same column. For RQG case
11000258898 EliminateUnnecessaryProject removed such a project after the
aggregate had been rewritten
into `Limit 1`, the plan above fell back to the nullable output slot of
the set operation, and the last
AdjustNullable pass reported
AdjustNullable convert slot 2004-04-28#55 from not-nullable to nullable
Fix:
Give the constant a fresh exprId only when it really shadows a column,
i.e. when the exprId is still
produced by the plan below; otherwise keep it:
* the constant gets a fresh exprId, and the references above are
refreshed with ExprIdRewriter, when the
aggregate still outputs the replaced column (its grouping key is kept
because all grouping keys are
constant) or when a project item re-defines a column which its child
still outputs. That is the shadow
the old code created, e.g. the RQG case 11000258898 of DORIS-28416.
* the constant keeps the exprId when the aggregate removed the constant
grouping key from its outputs:
the project item is then the only definition of the column, nothing
below references the exprId, and
consumers which rely on the identity of the column, e.g. the
materialized view matching, keep working.
The plan of such a query is byte identical to the plan before this
change, including the exprIds
(verified with the query of `agg_optimize_when_uniform/mv2_0`, for the
query side and for the MV side
which is generated with constant propagation disabled).
The sink also has to follow the fresh exprIds, otherwise it would
reference a slot which its child does
not output any more; only exprIds are remapped there, the name and the
data type declared by the sink are
kept (that is why the rule does not replace the slots of the sink by
literals).
A CTE is rewritten in separate subtree contexts (producer body and
consumers) and the consumers, which
map their slots to the producer outputs by exprId, are rewritten before
the producer. The renamed exprIds
are published per CTE (`StatementContext#producerExprIdRemap`), and
`RewriteCteChildren` refreshes both
maps of every consumer with the last exprId of the chain
(`ExprIdRewriter#lastExprId`).
Reproduction: ConstantPropagationFreshExprIdTest runs this rule alone,
so the project above the aggregate
is still visible in the plan, and asserts that it does not reuse the
exprId of the aggregate output and
that the plan above references the fresh exprIds.
ConstantPropagationTest used to pin the reused exprIds
in its constant project expectations; it now asserts fresh exprIds. Both
fail before this change.
### Release note
None
### Check List (For Author)
- Test: Unit Test
- ConstantPropagationFreshExprIdTest (new): fails before with "the
constant project reuses the exprId
of the aggregate output".
- ConstantPropagationTest: expectations updated, fails before with "the
constant project item must not
reuse the exprId of the replaced output slot".
- 31 related FE unit test classes (constant propagation, constant cte,
set operation, column pruning,
fd, uniform, sql plan suite, runtime filter, ...) rerun: 385 tests, 0
failures.
- Behavior changed: Yes. A constant materialized above an aggregate has
its own exprId and the plan above
references it, so one exprId no longer describes two different columns.
- Does this need documentation: No
---
.../org/apache/doris/nereids/StatementContext.java | 38 ++-
.../nereids/rules/rewrite/ConstantPropagation.java | 82 +++++-
.../nereids/rules/rewrite/ExprIdRewriter.java | 139 ++++++----
.../nereids/rules/rewrite/RewriteCteChildren.java | 89 ++++++-
.../ConstantPropagationFreshExprIdTest.java | 288 +++++++++++++++++++++
.../rules/rewrite/ConstantPropagationTest.java | 154 ++++++++++-
.../rewrite/CteConsumerOutputsRefresherTest.java | 107 ++++++++
.../nereids/rules/rewrite/ExprIdRewriterTest.java | 69 +++++
8 files changed, 894 insertions(+), 72 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 9f372233711..cd71830ed64 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
@@ -197,6 +197,11 @@ public class StatementContext implements Closeable {
private final Map<CTEId, List<Pair<Multimap<Slot, Slot>, Group>>>
cteIdToConsumerGroup = new HashMap<>();
private final Map<CTEId, LogicalPlan> rewrittenCteProducer = new
HashMap<>();
private final Map<CTEId, LogicalPlan> rewrittenCteConsumer = new
HashMap<>();
+ // The exprIds which a rule renamed while it rewrote the body of a CTE
producer, keyed by the previous exprId.
+ // The consumers of that CTE map their slots to the outputs of the
producer by exprId and are rewritten before
+ // the producer subtree, so every rule which renames an output of a
producer body has to publish it here, and
+ // RewriteCteChildren refreshes the consumer maps with it.
ConstantPropagation is currently the only publisher.
+ private final Map<CTEId, Map<ExprId, ExprId>> producerExprIdRemap = new
HashMap<>();
private final Set<String> viewDdlSqlSet = Sets.newHashSet();
private final SqlCacheContext sqlCacheContext;
@@ -958,6 +963,19 @@ public class StatementContext implements Closeable {
return rewrittenCteConsumer;
}
+ /**
+ * Record that a rule renamed exprIds while it rewrote the body of a CTE
producer. Rules may rename more than
+ * once, so the mappings are accumulated instead of replaced, and a reader
has to follow the chain to its end.
+ */
+ public void addProducerExprIdRemap(CTEId cteId, Map<ExprId, ExprId>
renamedExprIds) {
+ producerExprIdRemap.computeIfAbsent(cteId, k -> new
HashMap<>()).putAll(renamedExprIds);
+ }
+
+ /** The exprIds renamed while rewriting the body of a CTE producer, keyed
by their previous exprId. */
+ public Map<ExprId, ExprId> getProducerExprIdRemap(CTEId cteId) {
+ return producerExprIdRemap.getOrDefault(cteId, Collections.emptyMap());
+ }
+
/** Clear CTE-related rewrite and memo state before rebuilding it from a
new plan tree. */
public void clearCteEnvironment() {
cteIdToConsumers.clear();
@@ -968,6 +986,7 @@ public class StatementContext implements Closeable {
cteIdToConsumerGroup.clear();
rewrittenCteProducer.clear();
rewrittenCteConsumer.clear();
+ producerExprIdRemap.clear();
}
/**
@@ -982,7 +1001,8 @@ public class StatementContext implements Closeable {
new HashMap<>(consumerIdToLimitRows),
copyMapOfLists(cteIdToConsumerGroup),
new HashMap<>(rewrittenCteProducer),
- new HashMap<>(rewrittenCteConsumer));
+ new HashMap<>(rewrittenCteConsumer),
+ copyMapOfMaps(producerExprIdRemap));
}
/** Restore CTE-related environment from snapshot. */
@@ -1010,6 +1030,17 @@ public class StatementContext implements Closeable {
rewrittenCteConsumer.clear();
rewrittenCteConsumer.putAll(snapshot.rewrittenCteConsumer);
+
+ producerExprIdRemap.clear();
+ producerExprIdRemap.putAll(snapshot.producerExprIdRemap);
+ }
+
+ private static <K, V, W> Map<K, Map<V, W>> copyMapOfMaps(Map<K, Map<V, W>>
source) {
+ Map<K, Map<V, W>> copied = new HashMap<>();
+ for (Map.Entry<K, Map<V, W>> entry : source.entrySet()) {
+ copied.put(entry.getKey(), new HashMap<>(entry.getValue()));
+ }
+ return copied;
}
private static <K, V> Map<K, Set<V>> copyMapOfSets(Map<K, Set<V>> source) {
@@ -1038,6 +1069,7 @@ public class StatementContext implements Closeable {
private final Map<CTEId, List<Pair<Multimap<Slot, Slot>, Group>>>
cteIdToConsumerGroup;
private final Map<CTEId, LogicalPlan> rewrittenCteProducer;
private final Map<CTEId, LogicalPlan> rewrittenCteConsumer;
+ private final Map<CTEId, Map<ExprId, ExprId>> producerExprIdRemap;
/**
* cte related structures in StatementContext
@@ -1050,7 +1082,8 @@ public class StatementContext implements Closeable {
Map<RelationId, Long> consumerIdToLimitRows,
Map<CTEId, List<Pair<Multimap<Slot, Slot>, Group>>>
cteIdToConsumerGroup,
Map<CTEId, LogicalPlan> rewrittenCteProducer,
- Map<CTEId, LogicalPlan> rewrittenCteConsumer) {
+ Map<CTEId, LogicalPlan> rewrittenCteConsumer,
+ Map<CTEId, Map<ExprId, ExprId>> producerExprIdRemap) {
this.cteIdToConsumers = cteIdToConsumers;
this.cteIdToOutputIds = cteIdToOutputIds;
this.cteIdToProducer = cteIdToProducer;
@@ -1059,6 +1092,7 @@ public class StatementContext implements Closeable {
this.cteIdToConsumerGroup = cteIdToConsumerGroup;
this.rewrittenCteProducer = rewrittenCteProducer;
this.rewrittenCteConsumer = rewrittenCteConsumer;
+ this.producerExprIdRemap = producerExprIdRemap;
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagation.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagation.java
index 12570f95035..c1531addfa1 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagation.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagation.java
@@ -27,14 +27,17 @@ import
org.apache.doris.nereids.rules.expression.ExpressionRewriteContext;
import org.apache.doris.nereids.rules.expression.rules.FoldConstantRule;
import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.And;
+import org.apache.doris.nereids.trees.expressions.CTEId;
import org.apache.doris.nereids.trees.expressions.EqualPredicate;
import org.apache.doris.nereids.trees.expressions.EqualTo;
+import org.apache.doris.nereids.trees.expressions.ExprId;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.Match;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
import org.apache.doris.nereids.trees.expressions.Or;
import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.expressions.SlotReference;
+import org.apache.doris.nereids.trees.expressions.StatementScopeIdGenerator;
import org.apache.doris.nereids.trees.expressions.literal.BooleanLiteral;
import org.apache.doris.nereids.trees.expressions.literal.ComparableLiteral;
import org.apache.doris.nereids.trees.expressions.literal.Literal;
@@ -71,6 +74,7 @@ import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import java.util.stream.Collectors;
/**
* constant propagation, like: a = 10 and a + b > 30 => a = 10 and 10 + b > 30,
@@ -79,6 +83,17 @@ import java.util.Set;
* finally use the combine uniforms to replace this plan's expression's slot
with literals.
*/
public class ConstantPropagation extends DefaultPlanRewriter<CascadesContext>
implements CustomRewriter {
+ /**
+ * When this rule replaces a slot with a constant, the new slot describes
another value than the replaced
+ * slot: the nullable and the data type may differ. Reusing the exprId of
the replaced slot would make two
+ * different definitions share one exprId, and every rule which only
compares exprIds (e.g.
+ * EliminateUnnecessaryProject) would treat them as the same column.
+ *
+ * <p>So the constant slot gets a fresh exprId, and this map records
{@code replaced exprId -> fresh exprId}
+ * to refresh the references in the plan above.
+ */
+ private final Map<ExprId, ExprId> replacedExprIds = Maps.newHashMap();
+ private ExprIdRewriter exprIdRewriter;
@Override
public Plan rewriteRoot(Plan plan, JobContext jobContext) {
@@ -86,12 +101,33 @@ public class ConstantPropagation extends
DefaultPlanRewriter<CascadesContext> im
if (plan.containsType(LogicalApply.class)) {
return plan;
}
- return plan.accept(this, jobContext.getCascadesContext());
+ replacedExprIds.clear();
+ CascadesContext context = jobContext.getCascadesContext();
+ // a present current tree means this plan is the body of a CTE
producer, see RewriteCteChildren
+ Optional<CTEId> cteId = context.getCurrentTree();
+ exprIdRewriter = new ExprIdRewriter(new
ExprIdRewriter.ReplaceRule(replacedExprIds, true), jobContext);
+ Plan rewrittenPlan = plan.accept(this, context);
+ if (cteId.isPresent() && !replacedExprIds.isEmpty()) {
+ // The consumers of this CTE map their slots to the outputs of the
producer by exprId, and they are
+ // rewritten before the producer subtree, so publish the renamed
exprIds for RewriteCteChildren to
+ // refresh their maps. A rule may rename more than once, so the
mappings are accumulated.
+ context.getStatementContext().addProducerExprIdRemap(cteId.get(),
replacedExprIds);
+ }
+ return rewrittenPlan;
+ }
+
+ @Override
+ public Plan visit(Plan plan, CascadesContext context) {
+ plan = visitChildren(this, plan, context);
+ // the children may have replaced some slots by constants with fresh
exprIds, refresh the references of
+ // this node before rewriting it
+ return exprIdRewriter.rewriteExpr(plan, replacedExprIds);
}
@Override
public Plan visitLogicalFilter(LogicalFilter<? extends Plan> filter,
CascadesContext context) {
filter = visitChildren(this, filter, context);
+ filter = (LogicalFilter<? extends Plan>)
exprIdRewriter.rewriteExpr(filter, replacedExprIds);
Expression oldPredicate = filter.getPredicate();
Expression newPredicate = replaceConstantsAndRewriteExpr(filter,
oldPredicate, context);
if (isExprEqualIgnoreOrder(oldPredicate, newPredicate)) {
@@ -105,6 +141,7 @@ public class ConstantPropagation extends
DefaultPlanRewriter<CascadesContext> im
@Override
public Plan visitLogicalHaving(LogicalHaving<? extends Plan> having,
CascadesContext context) {
having = visitChildren(this, having, context);
+ having = (LogicalHaving<? extends Plan>)
exprIdRewriter.rewriteExpr(having, replacedExprIds);
Expression oldPredicate = having.getPredicate();
Expression newPredicate = replaceConstantsAndRewriteExpr(having,
oldPredicate, context);
if (isExprEqualIgnoreOrder(oldPredicate, newPredicate)) {
@@ -118,14 +155,17 @@ public class ConstantPropagation extends
DefaultPlanRewriter<CascadesContext> im
@Override
public Plan visitLogicalProject(LogicalProject<? extends Plan> project,
CascadesContext context) {
project = visitChildren(this, project, context);
+ project = (LogicalProject<? extends Plan>)
exprIdRewriter.rewriteExpr(project, replacedExprIds);
ExpressionRewriteContext rewriteContext = new
ExpressionRewriteContext(project, context);
Pair<ImmutableEqualSet<Slot>, Map<Slot, Literal>> childEqualTrait =
getChildEqualSetAndConstants(project, rewriteContext);
ImmutableList.Builder<NamedExpression> newProjectsBuilder
=
ImmutableList.builderWithExpectedSize(project.getProjects().size());
+ Set<ExprId> childOutputExprIds = project.child().getOutput().stream()
+ .map(Slot::getExprId).collect(Collectors.toSet());
for (NamedExpression expr : project.getProjects()) {
newProjectsBuilder.add(replaceNameExpressionConstants(
- expr, rewriteContext, childEqualTrait.first,
childEqualTrait.second));
+ expr, rewriteContext, childEqualTrait.first,
childEqualTrait.second, childOutputExprIds));
}
List<NamedExpression> newProjects = newProjectsBuilder.build();
@@ -135,6 +175,7 @@ public class ConstantPropagation extends
DefaultPlanRewriter<CascadesContext> im
@Override
public Plan visitLogicalSort(LogicalSort<? extends Plan> sort,
CascadesContext context) {
sort = visitChildren(this, sort, context);
+ sort = (LogicalSort<? extends Plan>) exprIdRewriter.rewriteExpr(sort,
replacedExprIds);
ExpressionRewriteContext rewriteContext = new
ExpressionRewriteContext(sort, context);
Pair<ImmutableEqualSet<Slot>, Map<Slot, Literal>> childEqualTrait
= getChildEqualSetAndConstants(sort, rewriteContext);
@@ -163,6 +204,7 @@ public class ConstantPropagation extends
DefaultPlanRewriter<CascadesContext> im
@Override
public Plan visitLogicalAggregate(LogicalAggregate<? extends Plan>
aggregate, CascadesContext context) {
aggregate = visitChildren(this, aggregate, context);
+ aggregate = (LogicalAggregate<? extends Plan>)
exprIdRewriter.rewriteExpr(aggregate, replacedExprIds);
ExpressionRewriteContext rewriteContext = new
ExpressionRewriteContext(aggregate, context);
Pair<ImmutableEqualSet<Slot>, Map<Slot, Literal>> childEqualTrait =
getChildEqualSetAndConstants(aggregate, rewriteContext);
@@ -192,7 +234,6 @@ public class ConstantPropagation extends
DefaultPlanRewriter<CascadesContext> im
List<NamedExpression> newOutputExprs =
Lists.newArrayListWithExpectedSize(oldOutputExprs.size());
ImmutableList.Builder<NamedExpression> projectBuilder
= ImmutableList.builderWithExpectedSize(oldOutputExprs.size());
-
boolean containsConstantOutput = false;
// after normal agg, group by expressions and output expressions are
slots,
@@ -208,7 +249,13 @@ public class ConstantPropagation extends
DefaultPlanRewriter<CascadesContext> im
newOutputExprs.add((NamedExpression) newOutputExpr);
}
- if (replacedExpr.isConstant()) {
+ if (replacedExpr.isConstant() && !replacedExpr.equals(expr)) {
+ // The constant is materialized above the aggregate, but it
keeps the exprId of the aggregate
+ // output instead of getting a fresh one and refreshing the
plan above: when the aggregate still
+ // outputs that column, the value is the same constant, and
when it removed the column, this
+ // project item is the only definition of the exprId. Renaming
the exprId would break the
+ // consumers which rely on the identity of the column, e.g.
the materialized view matching, and
+ // would make the plan above be rewritten for nothing.
projectBuilder.add(new Alias(expr.getExprId(), replacedExpr,
expr.getName()));
containsConstantOutput = true;
} else {
@@ -218,6 +265,7 @@ public class ConstantPropagation extends
DefaultPlanRewriter<CascadesContext> im
}
if (newGroupByExprs.equals(oldGroupByExprs) &&
newOutputExprs.equals(oldOutputExprs)) {
+ // the aggregate is not changed, the plan above keeps referencing
the output slots of the aggregate
return aggregate;
}
@@ -238,6 +286,7 @@ public class ConstantPropagation extends
DefaultPlanRewriter<CascadesContext> im
// update: BE not support nested loop mark join, so mark join
condition need to be an equation,
// but constant propagation may rewrite an equal to TRUE/FALSE/NULL,
so we don't rewrite mark join condition.
join = visitChildren(this, join, context);
+ join = (LogicalJoin<? extends Plan, ? extends Plan>)
exprIdRewriter.rewriteExpr(join, replacedExprIds);
if (join.getJoinType().isAsofJoin()) {
return join;
}
@@ -295,7 +344,7 @@ public class ConstantPropagation extends
DefaultPlanRewriter<CascadesContext> im
// // the select will add a parent plan: result sink. the result sink
contains a output slot reference, and its
// // data type is varchar(30), but if replace the slot reference
with a varchar literal '1',
// // then the data type info varchar(30) will lost, because varchar
literal '1' data type is always varchar(1),
- // // so t's column will get a error type. so we don't rewrite logical
sink then.
+ // // so t's column will get a error type. so we don't replace the
slots of the sink by literals.
// Pair<ImmutableEqualSet<Slot>, Map<Slot, Literal>> childEqualTrait
// = getChildEqualSetAndConstants(sink, context);
// List<NamedExpression> newOutputExprs =
sink.getOutputExprs().stream()
@@ -303,7 +352,10 @@ public class ConstantPropagation extends
DefaultPlanRewriter<CascadesContext> im
// replaceNameExpressionConstants(expr, context,
childEqualTrait.first, childEqualTrait.second))
// .collect(ImmutableList.toImmutableList());
// return newOutputExprs.equals(sink.getOutputExprs()) ? sink :
sink.withOutputExprs(newOutputExprs);
- return sink;
+ // The sink still has to follow the fresh exprIds of the constants
materialized below it, otherwise it
+ // would reference a slot which its child does not output any more.
Only the exprId is remapped here, so
+ // the name and the data type declared by the sink are kept.
+ return (LogicalSink<? extends Plan>) exprIdRewriter.rewriteExpr(sink,
replacedExprIds);
}
/**
@@ -350,8 +402,7 @@ public class ConstantPropagation extends
DefaultPlanRewriter<CascadesContext> im
// process NameExpression
private NamedExpression replaceNameExpressionConstants(NamedExpression
expr, ExpressionRewriteContext context,
- ImmutableEqualSet<Slot> equalSet, Map<Slot, Literal> constants) {
-
+ ImmutableEqualSet<Slot> equalSet, Map<Slot, Literal> constants,
Set<ExprId> childOutputExprIds) {
// if a project item is a slot reference, and the slot equals to a
constant value, don't rewrite it.
// because rule `EliminateUnnecessaryProject ` can eliminate a project
when the project's output slots equal to
// its child's output slots.
@@ -371,12 +422,21 @@ public class ConstantPropagation extends
DefaultPlanRewriter<CascadesContext> im
return expr;
}
+ // the item is an Alias which is not a plain slot and not a cast of
one: replacing the constants of an
+ // Alias always yields an Alias again, so only its expression may
change
Expression newExpr = replaceConstants(expr, false, context, equalSet,
constants);
- if (newExpr instanceof NamedExpression) {
+ if (!(newExpr instanceof Alias) || newExpr.equals(expr)
+ || !childOutputExprIds.contains(expr.getExprId())) {
+ // unchanged, or the child does not produce the column any more,
then this project item is the only
+ // definition of the exprId and it is kept
return (NamedExpression) newExpr;
- } else {
- return new Alias(expr.getExprId(), newExpr, expr.getName());
}
+ // the child still produces this column, so the project item would
shadow it: materialize the constant with
+ // a fresh exprId and refresh the references above, see replacedExprIds
+ Alias replacedAlias = (Alias) newExpr;
+ ExprId freshExprId = StatementScopeIdGenerator.newExprId();
+ replacedExprIds.put(expr.getExprId(), freshExprId);
+ return new Alias(freshExprId, replacedAlias.child(0),
replacedAlias.getName());
}
private Expression replaceConstants(Expression expression, boolean
useInnerInfer, ExpressionRewriteContext context,
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ExprIdRewriter.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ExprIdRewriter.java
index 24ed8e1f068..4dcadc7e2f4 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ExprIdRewriter.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ExprIdRewriter.java
@@ -20,18 +20,18 @@ package org.apache.doris.nereids.rules.rewrite;
import org.apache.doris.nereids.jobs.JobContext;
import org.apache.doris.nereids.pattern.Pattern;
import org.apache.doris.nereids.rules.Rule;
-import org.apache.doris.nereids.rules.expression.ExpressionPatternMatcher;
-import org.apache.doris.nereids.rules.expression.ExpressionPatternRuleFactory;
import org.apache.doris.nereids.rules.expression.ExpressionRewrite;
+import org.apache.doris.nereids.rules.expression.ExpressionRewriteContext;
+import org.apache.doris.nereids.rules.expression.ExpressionRewriteRule;
import org.apache.doris.nereids.rules.expression.ExpressionRuleExecutor;
-import org.apache.doris.nereids.rules.expression.ExpressionRuleType;
import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.ExprId;
import org.apache.doris.nereids.trees.expressions.Expression;
-import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
import org.apache.doris.nereids.trees.expressions.SlotReference;
import
org.apache.doris.nereids.trees.expressions.visitor.DefaultExpressionRewriter;
import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.logical.LogicalSink;
import com.google.common.collect.ImmutableList;
@@ -44,11 +44,43 @@ public class ExprIdRewriter extends ExpressionRewrite {
private final JobContext jobContext;
public ExprIdRewriter(ReplaceRule replaceRule, JobContext jobContext) {
- super(new
ExpressionRuleExecutor(ImmutableList.of(bottomUp(replaceRule))));
+ super(new ExpressionRuleExecutor(ImmutableList.of(new
ReplaceExprIds(replaceRule))));
rules = buildRules();
this.jobContext = jobContext;
}
+ /** Replace the exprIds of one expression structurally, see
ReplaceRule#replaceExprIds. */
+ private static class ReplaceExprIds implements
ExpressionRewriteRule<ExpressionRewriteContext> {
+ private final ReplaceRule replaceRule;
+
+ ReplaceExprIds(ReplaceRule replaceRule) {
+ this.replaceRule = replaceRule;
+ }
+
+ @Override
+ public Expression rewrite(Expression expression,
ExpressionRewriteContext context) {
+ return replaceRule.replaceExprIds(expression);
+ }
+ }
+
+ /**
+ * Follow the replacement chain of an exprId to its end: an exprId may be
renamed more than once, e.g. by
+ * several rules or by several passes of the same rule, and only the last
exprId exists in the plan then.
+ */
+ public static ExprId lastExprId(Map<ExprId, ExprId> replaceMap, ExprId
exprId) {
+ ExprId lastExprId = replaceMap.get(exprId);
+ if (lastExprId == null) {
+ return exprId;
+ }
+ while (true) {
+ ExprId nextExprId = replaceMap.get(lastExprId);
+ if (nextExprId == null) {
+ return lastExprId;
+ }
+ lastExprId = nextExprId;
+ }
+ }
+
/**rewriteExpr*/
public Plan rewriteExpr(Plan plan, Map<ExprId, ExprId> replaceMap) {
if (replaceMap.isEmpty()) {
@@ -64,6 +96,25 @@ public class ExprIdRewriter extends ExpressionRewrite {
}
}
}
+ // ExpressionRewrite has a rule for the common sink types only, e.g.
there is none for the blackhole sink. A
+ // sink takes its output expressions from its child, so it must follow
the fresh exprIds of the child as
+ // well, otherwise it would keep expecting a slot which the rewritten
child does not output any more. So
+ // rewrite the output expressions of any sink type here.
+ if (plan instanceof LogicalSink) {
+ LogicalSink<? extends Plan> sink = (LogicalSink<? extends Plan>)
plan;
+ ExpressionRewriteContext context = new
ExpressionRewriteContext(sink, jobContext.getCascadesContext());
+ ImmutableList.Builder<NamedExpression> newOutputExprs
+ =
ImmutableList.builderWithExpectedSize(sink.getOutputExprs().size());
+ boolean changed = false;
+ for (NamedExpression outputExpr : sink.getOutputExprs()) {
+ NamedExpression newOutputExpr = (NamedExpression)
rewriter.rewrite(outputExpr, context);
+ changed |= !newOutputExpr.equals(outputExpr);
+ newOutputExprs.add(newOutputExpr);
+ }
+ if (changed) {
+ return sink.withOutputExprs(newOutputExprs.build());
+ }
+ }
return plan;
}
@@ -76,64 +127,38 @@ public class ExprIdRewriter extends ExpressionRewrite {
* e.g. replaceMap:{0:3, 1:6, 6:7}
* SlotReference:a#0 -> a#3, a#1 -> a#7
* */
- public static class ReplaceRule implements ExpressionPatternRuleFactory {
+ public static class ReplaceRule {
private static final DefaultExpressionRewriter<Map<ExprId, ExprId>>
SLOT_REPLACER =
new DefaultExpressionRewriter<Map<ExprId, ExprId>>() {
@Override
public Expression visitSlotReference(SlotReference slot,
Map<ExprId, ExprId> replaceMap) {
- ExprId newId = replaceMap.get(slot.getExprId());
- if (newId == null) {
+ if (replaceMap.get(slot.getExprId()) == null) {
return slot;
}
- ExprId lastId = newId;
- while (true) {
- newId = replaceMap.get(lastId);
- if (newId == null) {
- return slot.withExprId(lastId);
- } else {
- lastId = newId;
- }
- }
+ return slot.withExprId(lastExprId(replaceMap,
slot.getExprId()));
}
};
private static final DefaultExpressionRewriter<Map<ExprId, ExprId>>
SLOT_ALIAS_REPLACER =
new DefaultExpressionRewriter<Map<ExprId, ExprId>>() {
@Override
public Expression visitSlotReference(SlotReference slot,
Map<ExprId, ExprId> replaceMap) {
- ExprId newId = replaceMap.get(slot.getExprId());
- if (newId == null) {
+ if (replaceMap.get(slot.getExprId()) == null) {
return slot;
}
- ExprId lastId = newId;
- while (true) {
- newId = replaceMap.get(lastId);
- if (newId == null) {
- return slot.withExprId(lastId);
- } else {
- lastId = newId;
- }
- }
+ return slot.withExprId(lastExprId(replaceMap,
slot.getExprId()));
}
@Override
public Expression visitAlias(Alias alias, Map<ExprId,
ExprId> replaceMap) {
- ExprId newId = replaceMap.get(alias.getExprId());
- if (newId == null) {
+ if (replaceMap.get(alias.getExprId()) == null) {
return alias;
}
- ExprId lastId = newId;
- while (true) {
- newId = replaceMap.get(lastId);
- if (newId == null) {
- return alias.withExprId(lastId);
- } else {
- lastId = newId;
- }
- }
+ return alias.withExprId(lastExprId(replaceMap,
alias.getExprId()));
}
};
private final Map<ExprId, ExprId> replaceMap;
+ private final DefaultExpressionRewriter<Map<ExprId, ExprId>>
structuralReplacer;
private DefaultExpressionRewriter<Map<ExprId, ExprId>> replacer;
/**
@@ -146,24 +171,36 @@ public class ExprIdRewriter extends ExpressionRewrite {
} else {
replacer = SLOT_REPLACER;
}
+ // children first: the id replacement of a node must not stop the
traversal, e.g. an Alias keeps its own
+ // exprId while a slot of the expression below it is renamed. The
replacer is applied to the node itself
+ // only, its children are already rewritten here, so the whole
expression is visited once.
+ this.structuralReplacer = new
DefaultExpressionRewriter<Map<ExprId, ExprId>>() {
+ @Override
+ public Expression visit(Expression expr, Map<ExprId, ExprId>
replaceMap) {
+ Expression withRewrittenChildren = rewriteChildren(this,
expr, replaceMap);
+ if (withRewrittenChildren instanceof SlotReference
+ || (rewriteAlias && withRewrittenChildren
instanceof Alias)) {
+ return withRewrittenChildren.accept(replacer,
replaceMap);
+ }
+ return withRewrittenChildren;
+ }
+ };
}
public ReplaceRule(Map<ExprId, ExprId> replaceMap) {
this(replaceMap, false);
}
- @Override
- public List<ExpressionPatternMatcher<? extends Expression>>
buildRules() {
- return ImmutableList.of(
- matchesType(SlotReference.class).thenApply(ctx -> {
- Slot slot = ctx.expr;
- return slot.accept(replacer, replaceMap);
- }).toRule(ExpressionRuleType.EXPR_ID_REWRITE_REPLACE),
- matchesType(Alias.class).thenApply(ctx -> {
- Alias alias = ctx.expr;
- return alias.accept(replacer, replaceMap);
- }).toRule(ExpressionRuleType.EXPR_ID_REWRITE_REPLACE)
- );
+ /**
+ * Replace the exprIds of the slots (and of the aliases, when this
rule was created for them) in one
+ * expression. Unlike the bottom up expression rewriter this enters
every branch, including the branches
+ * which require short circuit evaluation, e.g. the assignments of
MERGE INTO: replacing an exprId only
+ * rebuilds the expression and never evaluates it, and a reference
which keeps an old exprId would point to
+ * a slot which the rewritten child does not output any more.
+ */
+ public Expression replaceExprIds(Expression expression) {
+ return expression.accept(structuralReplacer, replaceMap);
}
+
}
}
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 c957a9e853f..fe68decaa5d 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
@@ -24,6 +24,7 @@ import org.apache.doris.nereids.jobs.executor.Rewriter;
import org.apache.doris.nereids.jobs.rewrite.RewriteJob;
import org.apache.doris.nereids.properties.PhysicalProperties;
import org.apache.doris.nereids.trees.expressions.CTEId;
+import org.apache.doris.nereids.trees.expressions.ExprId;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
import org.apache.doris.nereids.trees.expressions.Slot;
@@ -44,10 +45,15 @@ import org.apache.doris.nereids.util.ExpressionUtils;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.LinkedHashMultimap;
+import com.google.common.collect.Multimap;
import com.google.common.collect.Sets;
+import java.util.HashMap;
import java.util.HashSet;
+import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Map;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.Set;
@@ -99,22 +105,93 @@ public class RewriteCteChildren extends
DefaultPlanRewriter<CascadesContext> imp
cascadesContext.addPlanProcesses(outerCascadesCtx.getPlanProcesses());
cascadesContext.getStatementContext().getRewrittenCteConsumer().put(cteAnchor.getCteId(),
outer);
}
+ Set<LogicalCTEConsumer> cteConsumers = collectCteConsumers(outer,
cteAnchor.getCteId());
+ cascadesContext.getCteIdToConsumers().put(cteAnchor.getCteId(),
cteConsumers);
+ if (cteConsumers.isEmpty()) {
+ return outer;
+ }
+ Plan producer = cteAnchor.child(0).accept(this, cascadesContext);
+ // ConstantPropagation is currently the only rule which renames the
exprIds of a producer output, and it
+ // publishes what it renamed (see
StatementContext#addProducerExprIdRemap); a rule which renames an output
+ // of a producer body has to do the same, otherwise the consumers,
which are rewritten before the producer,
+ // keep mapping to an exprId which the producer does not output any
more. A rule may rename more than once,
+ // so the chain is followed to its end.
+ Map<ExprId, ExprId> producerExprIdRemap =
cascadesContext.getStatementContext()
+ .getProducerExprIdRemap(cteAnchor.getCteId());
+ if (!producerExprIdRemap.isEmpty()) {
+ Map<ExprId, Slot> producerOutputSlots = new HashMap<>();
+ for (Slot producerOutputSlot : producer.getOutput()) {
+ // a CTE may expose the same slot more than once, e.g. `SELECT
k, k FROM t`: both positions are the
+ // same producer slot, so the first one is enough for the
lookup
+
producerOutputSlots.putIfAbsent(producerOutputSlot.getExprId(),
producerOutputSlot);
+ }
+ outer = (LogicalPlan) outer.<Plan, Void>accept(new
CteConsumerOutputsRefresher(cteAnchor.getCteId(),
+ producerExprIdRemap, producerOutputSlots), null);
+
cascadesContext.getStatementContext().getRewrittenCteConsumer().put(cteAnchor.getCteId(),
outer);
+ cascadesContext.getCteIdToConsumers()
+ .put(cteAnchor.getCteId(), collectCteConsumers(outer,
cteAnchor.getCteId()));
+ }
+ return cteAnchor.withChildren(producer, outer);
+ }
+
+ private static Set<LogicalCTEConsumer> collectCteConsumers(Plan outer,
CTEId cteId) {
Set<LogicalCTEConsumer> cteConsumers = Sets.newHashSet();
outer.foreach(p -> {
if (p instanceof LogicalCTEConsumer) {
LogicalCTEConsumer logicalCTEConsumer = (LogicalCTEConsumer) p;
- if
(logicalCTEConsumer.getCteId().equals(cteAnchor.getCteId())) {
+ if (logicalCTEConsumer.getCteId().equals(cteId)) {
cteConsumers.add(logicalCTEConsumer);
}
}
return false;
});
- cascadesContext.getCteIdToConsumers().put(cteAnchor.getCteId(),
cteConsumers);
- if (cteConsumers.isEmpty()) {
- return outer;
+ return cteConsumers;
+ }
+
+ /**
+ * Refresh the producer slots of the two output maps of the consumers of a
CTE with the exprIds the rules of the
+ * producer body published, see StatementContext#getProducerExprIdRemap.
+ */
+ static class CteConsumerOutputsRefresher extends DefaultPlanRewriter<Void>
{
+ private final CTEId cteId;
+ private final Map<ExprId, ExprId> producerExprIdRemap;
+ private final Map<ExprId, Slot> producerOutputSlots;
+
+ CteConsumerOutputsRefresher(CTEId cteId, Map<ExprId, ExprId>
producerExprIdRemap,
+ Map<ExprId, Slot> producerOutputSlots) {
+ this.cteId = cteId;
+ this.producerExprIdRemap = producerExprIdRemap;
+ this.producerOutputSlots = producerOutputSlots;
+ }
+
+ @Override
+ public Plan visitLogicalCTEConsumer(LogicalCTEConsumer cteConsumer,
Void context) {
+ if (!cteConsumer.getCteId().equals(cteId)) {
+ return cteConsumer;
+ }
+ Map<Slot, Slot> consumerToProducerOutputMap = new
LinkedHashMap<>();
+ Multimap<Slot, Slot> producerToConsumerOutputMap =
LinkedHashMultimap.create();
+ for (Map.Entry<Slot, Slot> entry :
cteConsumer.getConsumerToProducerOutputMap().entrySet()) {
+ Slot newProducerSlot = refreshProducerSlot(entry.getValue());
+ consumerToProducerOutputMap.put(entry.getKey(),
newProducerSlot);
+ for (Slot consumerSlot :
cteConsumer.getProducerToConsumerOutputMap().get(entry.getValue())) {
+ producerToConsumerOutputMap.put(newProducerSlot,
consumerSlot);
+ }
+ }
+ return cteConsumer.withTwoMaps(consumerToProducerOutputMap,
producerToConsumerOutputMap);
+ }
+
+ private Slot refreshProducerSlot(Slot producerSlot) {
+ ExprId lastExprId = ExprIdRewriter.lastExprId(producerExprIdRemap,
producerSlot.getExprId());
+ if (lastExprId.equals(producerSlot.getExprId())) {
+ return producerSlot;
+ }
+ // the column may be pruned by the rewrite of the producer body
after it was renamed, e.g. by
+ // ColumnPruning when the plan above the consumer stopped reading
it; such a column has no counterpart
+ // in the producer output, so its entry stays as it is
+ Slot newProducerSlot = producerOutputSlots.get(lastExprId);
+ return newProducerSlot != null ? newProducerSlot : producerSlot;
}
- Plan producer = cteAnchor.child(0).accept(this, cascadesContext);
- return cteAnchor.withChildren(producer, outer);
}
@Override
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationFreshExprIdTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationFreshExprIdTest.java
new file mode 100644
index 00000000000..674723d15a5
--- /dev/null
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationFreshExprIdTest.java
@@ -0,0 +1,288 @@
+// 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.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.nereids.trees.expressions.Alias;
+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.expressions.SlotReference;
+import org.apache.doris.nereids.trees.expressions.literal.IntegerLiteral;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
+import org.apache.doris.nereids.trees.plans.logical.LogicalCTEConsumer;
+import org.apache.doris.nereids.trees.plans.logical.LogicalCTEProducer;
+import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
+import org.apache.doris.nereids.util.PlanChecker;
+import org.apache.doris.utframe.TestWithFeService;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * The project which ConstantPropagation materializes above an aggregate must
not re-label a column which the plan
+ * below defines with another value.
+ *
+ * <p>A constant grouping key which the aggregate keeps is passed through as
the slot of the aggregate, and a
+ * constant grouping key which the aggregate removes keeps the exprId it
already had, so nothing below defines that
+ * exprId any more. The project item used to reuse the exprId of the removed
aggregate output while other rules
+ * later brought a subtree which outputs that exprId back below it, and rules
which compare exprIds only, e.g.
+ * EliminateUnnecessaryProject, then removed the constant project. RQG case
11000258898 reached such a plan and the
+ * last AdjustNullable pass reported "AdjustNullable convert slot
2004-04-28#xx from not-nullable to nullable".
+ */
+public class ConstantPropagationFreshExprIdTest extends TestWithFeService {
+ private static final String SQL_11000258898 = "select '2004-04-28',
'2014-07-07'"
+ + " intersect distinct"
+ + " select col_datetime_3__undef_signed_not_null,
col_date_undef_signed_not_null"
+ + " from
table_100_undef_partitions2_keys3_properties4_distributed_by52"
+ + " where col_decimal_12_2__undef_signed = 94.0212"
+ + " union distinct"
+ + " select col_datetime_3__undef_signed,
col_datetime_3__undef_signed_not_null"
+ + " from
table_100_undef_partitions2_keys3_properties4_distributed_by54"
+ + " where col_datetime_3__undef_signed = '2011-11-15 13:33:05'"
+ + " intersect"
+ + " select col_date_undef_signed_not_null,
col_date_undef_signed_not_null"
+ + " from
table_100_undef_partitions2_keys3_properties4_distributed_by53"
+ + " intersect"
+ + " select col_datetime_3__undef_signed_not_null,
col_date_undef_signed_not_null"
+ + " from
table_100_undef_partitions2_keys3_properties4_distributed_by54 ;";
+
+ private static final String SQL_11000259585 = "select * from ((select
'vZRgkfajsD', 'cKToNmsxok' except (select col_char_25__undef_signed_not_null,
col_char_40__undef_signed from
table_100_undef_partitions2_keys3_properties4_distributed_by54 where
col_date_undef_signed = '2016-09-20' order by 1, 2 desc limit 1, 8) union
distinct select col_varchar_25__undef_signed, col_char_25__undef_signed from
table_100_undef_partitions2_keys3_properties4_distributed_by53 where
col_tinyint_undef_si [...]
+
+ private static final String SQL_MATERIALIZED_CTE = "WITH c AS (SELECT k,
v, count(*) AS cnt"
+ + " FROM constant_propagation_fresh_expr_id_t WHERE k = 5 AND v =
7 GROUP BY k, v)"
+ + " SELECT l.k, l.v, l.cnt, r.k, r.v, r.cnt FROM c l CROSS JOIN c
r";
+
+ private static final String SQL_MATERIALIZED_CTE_CONSTANT_COLUMN
+ = "WITH c AS (SELECT v, k + 0 AS x, count(*) AS cnt"
+ + " FROM constant_propagation_fresh_expr_id_t WHERE k = 5 GROUP BY
v, k)"
+ + " SELECT l.v, l.x, l.cnt, r.v, r.x, r.cnt FROM c l CROSS JOIN c
r";
+
+ private static final String SQL_MATERIALIZED_CTE_REPEATED_SLOT
+ = "WITH c AS (SELECT k, v, v, count(*) AS cnt"
+ + " FROM constant_propagation_fresh_expr_id_t WHERE k = 5 AND v =
7 GROUP BY k, v)"
+ + " SELECT l.*, r.* FROM c l CROSS JOIN c r";
+
+ @Override
+ protected void runBeforeAll() throws Exception {
+ createDatabase("test");
+ createTable("create table test.constant_propagation_fresh_expr_id_t(k
int null, v int null)"
+ + " distributed by hash(k) properties('replication_num' =
'1');");
+ createTable("create table
test.table_100_undef_partitions2_keys3_properties4_distributed_by52 ("
+ + " col_datetime_3__undef_signed_not_null datetime(3) not
null,"
+ + " col_datetime_3__undef_signed datetime(3) null,"
+ + " col_datetime_undef_signed datetime null,"
+ + " col_datetime_undef_signed_not_null datetime not null,"
+ + " col_date_undef_signed_not_null date not null,"
+ + " col_date_undef_signed date null,"
+ + " col_decimal_12_2__undef_signed decimal(12,2) null,"
+ + " col_decimal_32_6__undef_signed decimal(32,6) null)"
+ + " distributed by hash(col_datetime_3__undef_signed)
properties('replication_num' = '1');");
+ createTable("create table
test.table_100_undef_partitions2_keys3_properties4_distributed_by53 ("
+ + " col_date_undef_signed_not_null date not null,"
+ + " col_date_undef_signed date null,"
+ + " col_datetime_3__undef_signed datetime(3) null,"
+ + " col_datetime_3__undef_signed_not_null datetime(3) not
null,"
+ + " col_datetime_undef_signed datetime null,"
+ + " col_datetime_undef_signed_not_null datetime not null,"
+ + " col_varchar_25__undef_signed varchar(25) null,"
+ + " col_char_25__undef_signed_not_null char(25) not null,"
+ + " col_char_25__undef_signed char(25) null,"
+ + " col_char_40__undef_signed char(40) null,"
+ + " col_varchar_100__undef_signed varchar(100) null,"
+ + " col_tinyint_undef_signed_not_null tinyint not null,"
+ + " col_bigint_undef_signed_not_null bigint not null)"
+ + " distributed by hash(col_date_undef_signed_not_null)
properties('replication_num' = '1');");
+ createTable("create table
test.table_100_undef_partitions2_keys3_properties4_distributed_by54 ("
+ + " col_datetime_3__undef_signed datetime(3) null,"
+ + " col_datetime_3__undef_signed_not_null datetime(3) not
null,"
+ + " col_datetime_undef_signed datetime null,"
+ + " col_datetime_undef_signed_not_null datetime not null,"
+ + " col_date_undef_signed_not_null date not null,"
+ + " col_date_undef_signed date null,"
+ + " col_char_25__undef_signed_not_null char(25) not null,"
+ + " col_char_40__undef_signed char(40) null)"
+ + " distributed by hash(col_datetime_3__undef_signed)
properties('replication_num' = '1');");
+ createTable("create table
test.table_100_undef_partitions2_keys3_properties4_distributed_by5 ("
+ + " col_date_undef_signed_not_null date not null,"
+ + " col_datetime_undef_signed datetime null,"
+ + " col_datetime_undef_signed_not_null datetime not null,"
+ + " col_datetime_3__undef_signed datetime(3) null,"
+ + " col_datetime_3__undef_signed_not_null datetime(3) not
null,"
+ + " col_char_25__undef_signed char(25) null,"
+ + " col_char_25__undef_signed_not_null char(25) not null,"
+ + " col_varchar_100__undef_signed_not_null varchar(100) not
null,"
+ + " col_varchar_100__undef_signed varchar(100) null)"
+ + " distributed by hash(col_date_undef_signed_not_null)
properties('replication_num' = '1');");
+ connectContext.setDatabase("test");
+
connectContext.getSessionVariable().setDisableNereidsRules("PRUNE_EMPTY_PARTITION");
+ }
+
+ @Test
+ void testConstantAggregateProjectKeepsTheExprId() {
+ // run this rule alone, so the project it materializes above the
aggregate is still part of the plan
+ PlanChecker checker = PlanChecker.from(connectContext)
+ .analyze("select k, v from
constant_propagation_fresh_expr_id_t where k = 5 and v = 7 group by k, v")
+ .applyCustom(new ConstantPropagation());
+ Plan plan = checker.getPlan();
+ LogicalProject<?> constantProject = findConstantProject(plan);
+ Assertions.assertNotNull(constantProject, plan.treeString());
+ List<NamedExpression> items = constantProject.getProjects();
+ // the kept grouping key is still materialized as a constant above the
aggregate, with the exprId of the
+ // aggregate output: no fresh exprId and no refresh of the plan above
+ NamedExpression keptKey = items.get(0);
+ Assertions.assertTrue(keptKey instanceof Alias, plan.treeString());
+ Assertions.assertEquals(new IntegerLiteral(5), ((Alias)
keptKey).child());
+ Assertions.assertTrue(constantProject.child().getOutput().stream()
+ .anyMatch(slot ->
slot.getExprId().equals(keptKey.getExprId())),
+ "the kept grouping key keeps the exprId of the aggregate
output:\n" + plan.treeString());
+ // the removed grouping key is materialized with the exprId it already
had as well
+ NamedExpression removedKey = items.get(1);
+ Assertions.assertTrue(removedKey instanceof Alias, plan.treeString());
+ Assertions.assertEquals(new IntegerLiteral(7), ((Alias)
removedKey).child());
+ // the plan above references the outputs of the project, unchanged
+ Set<ExprId> projectOutputIds = constantProject.getOutput().stream()
+ .map(Slot::getExprId).collect(Collectors.toSet());
+ Set<ExprId> rootOutputIds = plan.getOutput().stream()
+ .map(Slot::getExprId).collect(Collectors.toSet());
+ Assertions.assertTrue(rootOutputIds.containsAll(projectOutputIds),
+ "the plan above does not reference the project outputs:\n" +
plan.treeString());
+ }
+
+ @Test
+ void testRqgCase11000258898() {
+ // the RQG case this PR was reported from: on the reported base it
tripped "AdjustNullable convert slot
+ // 2004-04-28#xx from not-nullable to nullable" during the rewrite, so
the rewrite and the planning of the
+ // query have to succeed
+ PlanChecker.from(connectContext).analyze(SQL_11000258898).rewrite();
+ PlanChecker.from(connectContext).checkPlannerResult(SQL_11000258898);
+ }
+
+ @Test
+ void testRqgCase11000259585() {
+ // the second case of the same RQG report, same failure mode: the
rewrite and the planning have to succeed
+ PlanChecker.from(connectContext).analyze(SQL_11000259585).rewrite();
+ PlanChecker.from(connectContext).checkPlannerResult(SQL_11000259585);
+ }
+
+ @Test
+ void testMaterializedCteConsumerMapsStayValid() throws Exception {
+ // the producer body and the consumers are rewritten in different
subtree contexts and the consumers are
+ // rewritten first, so the fresh exprIds of the producer outputs must
not cross the CTE boundary
+ Set<ExprId> analyzedProducerOutputIds = cteProducerOutputIds(
+
PlanChecker.from(connectContext).analyze(SQL_MATERIALIZED_CTE).getPlan());
+ Plan plan =
PlanChecker.from(connectContext).analyze(SQL_MATERIALIZED_CTE).rewrite().getPlan();
+ assertCteConsumerMapsStayValid(plan);
+ // constant propagation keeps the exprIds of the producer outputs, so
they are the same as in the analyzed
+ // plan and the consumer maps still resolve; a rule which renames them
has to refresh the maps, which the
+ // assertCteConsumerMapsStayValid above checks
+ Set<ExprId> rewrittenProducerOutputIds = cteProducerOutputIds(plan);
+ Assertions.assertEquals(analyzedProducerOutputIds.size(),
rewrittenProducerOutputIds.size(),
+ plan.treeString());
+ Assertions.assertEquals(analyzedProducerOutputIds,
rewrittenProducerOutputIds, plan.treeString());
+ // the query has to stay plannable
+
PlanChecker.from(connectContext).checkPlannerResult(SQL_MATERIALIZED_CTE);
+ }
+
+ @Test
+ void testMaterializedCteWithRepeatedOutputSlot() {
+ // a CTE may expose the same slot more than once (LogicalCTEConsumer
is multimap based), and the lookup of
+ // the producer slots must tolerate that instead of failing on the
duplicated exprId
+ Set<ExprId> analyzedProducerOutputIds = cteProducerOutputIds(
+
PlanChecker.from(connectContext).analyze(SQL_MATERIALIZED_CTE_REPEATED_SLOT).getPlan());
+ Plan plan = PlanChecker.from(connectContext)
+
.analyze(SQL_MATERIALIZED_CTE_REPEATED_SLOT).rewrite().getPlan();
+ assertCteConsumerMapsStayValid(plan);
+ Assertions.assertEquals(analyzedProducerOutputIds.size(),
cteProducerOutputIds(plan).size(), plan.treeString());
+
PlanChecker.from(connectContext).checkPlannerResult(SQL_MATERIALIZED_CTE_REPEATED_SLOT);
+ }
+
+ private static Set<ExprId> cteProducerOutputIds(Plan plan) {
+ Set<ExprId> producerOutputExprIds = new java.util.HashSet<>();
+ plan.foreach(node -> {
+ if (node instanceof LogicalCTEProducer) {
+ ((LogicalCTEProducer<?>) node).getOutput().forEach(slot ->
producerOutputExprIds.add(slot.getExprId()));
+ }
+ return false;
+ });
+ Assertions.assertFalse(producerOutputExprIds.isEmpty(),
plan.treeString());
+ return producerOutputExprIds;
+ }
+
+ /** Every producer slot a CTE consumer maps to has to be an output slot of
the producer. */
+ private static void assertCteConsumerMapsStayValid(Plan plan) {
+ Set<ExprId> producerOutputExprIds = new java.util.HashSet<>();
+ Set<LogicalCTEConsumer> consumers = new java.util.HashSet<>();
+ plan.foreach(node -> {
+ if (node instanceof LogicalCTEProducer) {
+ ((LogicalCTEProducer<?>) node).getOutput().forEach(slot ->
producerOutputExprIds.add(slot.getExprId()));
+ } else if (node instanceof LogicalCTEConsumer) {
+ consumers.add((LogicalCTEConsumer) node);
+ }
+ return false;
+ });
+ Assertions.assertFalse(consumers.isEmpty(), plan.treeString());
+ for (LogicalCTEConsumer consumer : consumers) {
+ for (Slot producerSlot :
consumer.getProducerToConsumerOutputMap().keySet()) {
+
Assertions.assertTrue(producerOutputExprIds.contains(producerSlot.getExprId()),
+ "the consumer map of " + consumer.getRelationId() + "
references " + producerSlot
+ + " which the producer does not output: " +
plan.treeString());
+ }
+ for (Slot producerSlot :
consumer.getConsumerToProducerOutputMap().values()) {
+
Assertions.assertTrue(producerOutputExprIds.contains(producerSlot.getExprId()),
+ "the consumer map of " + consumer.getRelationId() + "
references " + producerSlot
+ + " which the producer does not output: " +
plan.treeString());
+ }
+ }
+ }
+
+ @Test
+ void testMaterializedCteKeepsConstantProducerOutput() {
+ // `k + 0` is uniform (k = 5), so it folds to a constant, and it is an
output column of the CTE producer:
+ // whether the rule materializes the constant or leaves the column
alone, the consumers have to keep
+ // mapping to an output which the producer really exposes
+ Set<ExprId> analyzedProducerOutputIds = cteProducerOutputIds(
+
PlanChecker.from(connectContext).analyze(SQL_MATERIALIZED_CTE_CONSTANT_COLUMN).getPlan());
+ Plan plan = PlanChecker.from(connectContext)
+
.analyze(SQL_MATERIALIZED_CTE_CONSTANT_COLUMN).rewrite().getPlan();
+ assertCteConsumerMapsStayValid(plan);
+ // the producer keeps its three output columns, whether they are
rewritten in place or renamed
+ Assertions.assertEquals(analyzedProducerOutputIds.size(),
cteProducerOutputIds(plan).size(), plan.treeString());
+
PlanChecker.from(connectContext).checkPlannerResult(SQL_MATERIALIZED_CTE_CONSTANT_COLUMN);
+ }
+
+ private static LogicalProject<?> findConstantProject(Plan plan) {
+ if (plan instanceof LogicalProject
+ && plan.child(0) instanceof LogicalAggregate
+ && ((LogicalProject<?>)
plan).getProjects().stream().anyMatch(item -> !(item instanceof
SlotReference))) {
+ return (LogicalProject<?>) plan;
+ }
+ for (Plan child : plan.children()) {
+ LogicalProject<?> found = findConstantProject(child);
+ if (found != null) {
+ return found;
+ }
+ }
+ return null;
+ }
+
+}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationTest.java
index 14b82090d18..eea5de6aa85 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ConstantPropagationTest.java
@@ -27,12 +27,14 @@ import org.apache.doris.nereids.trees.expressions.Add;
import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.Cast;
import org.apache.doris.nereids.trees.expressions.EqualTo;
+import org.apache.doris.nereids.trees.expressions.ExprId;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.GreaterThan;
import org.apache.doris.nereids.trees.expressions.IsNull;
import org.apache.doris.nereids.trees.expressions.MatchAny;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
import org.apache.doris.nereids.trees.expressions.Not;
+import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.expressions.SlotReference;
import org.apache.doris.nereids.trees.expressions.functions.agg.Sum;
import org.apache.doris.nereids.trees.expressions.literal.BigIntLiteral;
@@ -43,12 +45,14 @@ import
org.apache.doris.nereids.trees.expressions.literal.StringLiteral;
import org.apache.doris.nereids.trees.plans.JoinType;
import org.apache.doris.nereids.trees.plans.RelationId;
import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
+import org.apache.doris.nereids.trees.plans.logical.LogicalBlackholeSink;
import org.apache.doris.nereids.trees.plans.logical.LogicalFilter;
import org.apache.doris.nereids.trees.plans.logical.LogicalHaving;
import org.apache.doris.nereids.trees.plans.logical.LogicalJoin;
import org.apache.doris.nereids.trees.plans.logical.LogicalOlapScan;
import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
+import org.apache.doris.nereids.trees.plans.logical.LogicalSink;
import org.apache.doris.nereids.trees.plans.logical.LogicalSort;
import org.apache.doris.nereids.types.BigIntType;
import org.apache.doris.nereids.types.IntegerType;
@@ -67,6 +71,7 @@ import org.junit.jupiter.api.Test;
import java.util.List;
import java.util.Set;
+import java.util.stream.Collectors;
class ConstantPropagationTest {
@@ -361,7 +366,18 @@ class ConstantPropagationTest {
LogicalAggregate rewrittenAgg1 = (LogicalAggregate)
rewrittenProject1.child();
Assertions.assertEquals(expectGroupby1,
rewrittenAgg1.getGroupByExpressions());
Assertions.assertEquals(expectAggOutput1,
rewrittenAgg1.getOutputExpressions());
- Assertions.assertEquals(expectProjection1,
rewrittenProject1.getProjects());
+ // the constant outputs above the aggregate are materialized with a
fresh exprId, so that one exprId
+ // does not describe both the output slot of the aggregate and the
constant
+ List<NamedExpression> actualProjects1 =
rewrittenProject1.getProjects();
+ Assertions.assertEquals(expectProjection1.subList(0, 5),
actualProjects1.subList(0, 5));
+ // the aggregate removes these two constant grouping keys from its
outputs, so the project item is the only
+ // definition of their exprIds and the ids are kept
+ assertKeptConstantProjectItem(actualProjects1.get(5),
aggOutput.get(5).getExprId(),
+ new IntegerLiteral(1), aggOutput.get(5).getName());
+ Assertions.assertEquals(expectProjection1.get(6),
actualProjects1.get(6));
+ assertKeptConstantProjectItem(actualProjects1.get(7),
aggOutput.get(7).getExprId(),
+ new IntegerLiteral(11), aggOutput.get(7).getName());
+ Assertions.assertEquals(expectProjection1.subList(8, 10),
actualProjects1.subList(8, 10));
Set<Expression> conjunctions2 = ImmutableSet.of(
new EqualTo(studentId, new IntegerLiteral(3)),
@@ -396,7 +412,141 @@ class ConstantPropagationTest {
LogicalAggregate rewrittenAgg2 = (LogicalAggregate)
rewrittenProject2.child();
Assertions.assertEquals(expectGroupby2,
rewrittenAgg2.getGroupByExpressions());
Assertions.assertEquals(expectAggOutput2, rewrittenAgg2.getOutputs());
- Assertions.assertEquals(expectProjection2,
rewrittenProject2.getProjects());
+ List<NamedExpression> actualProjects2 =
rewrittenProject2.getProjects();
+ Assertions.assertEquals(expectProjection2.subList(0, 5),
actualProjects2.subList(0, 5));
+ // all grouping keys are constant, so the aggregate keeps the first
one; the constant is still materialized
+ // above it, with the exprId of the aggregate output
+ assertKeptConstantProjectItem(actualProjects2.get(5),
expectAggOutput2.get(5).getExprId(),
+ new IntegerLiteral(3), null);
+ // the other keys are removed from the aggregate, their exprIds are
kept
+ assertKeptConstantProjectItem(actualProjects2.get(6),
aggOutput.get(6).getExprId(),
+ new IntegerLiteral(3), aggOutput.get(6).getName());
+ assertKeptConstantProjectItem(actualProjects2.get(7),
aggOutput.get(7).getExprId(),
+ new IntegerLiteral(13), aggOutput.get(7).getName());
+ assertKeptConstantProjectItem(actualProjects2.get(8),
aggOutput.get(8).getExprId(),
+ new IntegerLiteral(13), aggOutput.get(8).getName());
+ assertKeptConstantProjectItem(actualProjects2.get(9),
aggOutput.get(9).getExprId(),
+ new IntegerLiteral(6), aggOutput.get(9).getName());
+ }
+
+ @Test
+ void testConstantGroupByKeyKeepsExprIdWhenTheAggregateDropsIt() {
+ // positive gate for the materialized view shape which this PR must
not change: the constant is materialized
+ // above the aggregate with the exprId of the aggregate output, no
fresh exprId and no refresh of the plan
+ // above, so the identity of the column is kept (the MV matching
relies on it)
+ LogicalFilter<LogicalOlapScan> filter = new
LogicalFilter<>(ImmutableSet.of(
+ new EqualTo(studentId, new IntegerLiteral(5))), student);
+ List<Expression> groupby = ImmutableList.of(studentId, studentGender);
+ List<NamedExpression> aggOutput = ImmutableList.of(
+ new Alias(new Sum(studentAge), "sum_age"),
+ studentGender,
+ studentId);
+ LogicalAggregate<LogicalFilter<LogicalOlapScan>> aggregate
+ = new LogicalAggregate<>(groupby, aggOutput, filter);
+
+ LogicalProject<?> rewrittenProject = (LogicalProject<?>)
executor.rewriteRoot(aggregate, jobContext);
+ NamedExpression constantItem = rewrittenProject.getProjects().get(2);
+ assertKeptConstantProjectItem(constantItem, studentId.getExprId(), new
IntegerLiteral(5), studentId.getName());
+ // the aggregate dropped the key, so nothing below the project defines
this column any more
+ Assertions.assertFalse(rewrittenProject.child().getOutput().stream()
+ .anyMatch(slot ->
slot.getExprId().equals(studentId.getExprId())));
+ }
+
+ @Test
+ public void testProjectItemShadowingAChildSlotGetsFreshExprId() {
+ // a project item which re-defines a column which its child still
outputs describes another value, so it
+ // must be materialized with a fresh exprId, and the sink above has to
follow that exprId
+ LogicalFilter<LogicalOlapScan> filter = new
LogicalFilter<>(ImmutableSet.of(
+ new EqualTo(studentId, new IntegerLiteral(5))), student);
+ LogicalProject<LogicalFilter<LogicalOlapScan>> project = new
LogicalProject<>(ImmutableList.of(
+ new Alias(studentId.getExprId(), new Add(studentId, new
IntegerLiteral(0)), studentId.getName()),
+ studentGender), filter);
+ List<NamedExpression> sinkOutputExprs = ImmutableList.of(studentId,
studentGender);
+ LogicalBlackholeSink<LogicalProject<LogicalFilter<LogicalOlapScan>>>
sink
+ = new LogicalBlackholeSink<>(sinkOutputExprs, project);
+
+ LogicalSink<?> rewrittenSink = (LogicalSink<?>)
executor.rewriteRoot(sink, jobContext);
+ // the child of the project still outputs studentId, so the constant
item got a fresh exprId
+ NamedExpression constantItem = ((LogicalProject<?>)
rewrittenSink.child()).getProjects().get(0);
+ Assertions.assertEquals(new IntegerLiteral(5), ((Alias)
constantItem).child());
+ Assertions.assertNotEquals(studentId.getExprId(),
constantItem.getExprId());
+ // and the sink follows it, otherwise it would reference a slot which
its child no longer outputs
+ Assertions.assertEquals(constantItem.getExprId(),
rewrittenSink.getOutputExprs().get(0).getExprId());
+ List<ExprId> childOutputIds =
rewrittenSink.child().getOutput().stream()
+ .map(Slot::getExprId).collect(Collectors.toList());
+ for (NamedExpression outputExpr : rewrittenSink.getOutputExprs()) {
+
Assertions.assertTrue(childOutputIds.contains(outputExpr.getExprId()),
+ "the sink expects " + outputExpr + " which its child does
not output: "
+ + rewrittenSink.child().getOutput());
+ }
+ }
+
+ @Test
+ public void testBlackholeSinkKeepsTheOutputExprIds() {
+ // `warm up select` is planned with a blackhole sink which takes its
output expressions from its child, so
+ // the sink has to follow the fresh exprIds of the constants
materialized below it, otherwise it expects a
+ // slot which the rewritten child does not output any more
+ LogicalFilter<LogicalOlapScan> filter = new
LogicalFilter<>(ImmutableSet.of(
+ new EqualTo(studentId, new IntegerLiteral(5)),
+ new EqualTo(studentGender, new StringLiteral("F"))), student);
+ // all grouping keys are constant, so the aggregate keeps the first
one (and drops the other), the constant
+ // above the kept key shadows it and must get a fresh exprId
+ List<Expression> groupby = ImmutableList.of(studentId, studentGender);
+ List<NamedExpression> aggOutput = ImmutableList.of(
+ new Alias(new Sum(studentAge), "sum_age"),
+ studentId,
+ studentGender);
+ LogicalAggregate<LogicalFilter<LogicalOlapScan>> aggregate
+ = new LogicalAggregate<>(groupby, aggOutput, filter);
+ List<NamedExpression> sinkOutputExprs =
ImmutableList.copyOf(aggregate.getOutput());
+ LogicalBlackholeSink<LogicalAggregate<LogicalFilter<LogicalOlapScan>>>
sink
+ = new LogicalBlackholeSink<>(sinkOutputExprs, aggregate);
+
+ LogicalSink<?> rewrittenSink = (LogicalSink<?>)
executor.rewriteRoot(sink, jobContext);
+ List<ExprId> childOutputIds =
rewrittenSink.child().getOutput().stream()
+
.map(Slot::getExprId).collect(java.util.stream.Collectors.toList());
+ for (NamedExpression outputExpr : rewrittenSink.getOutputExprs()) {
+
Assertions.assertTrue(childOutputIds.contains(outputExpr.getExprId()),
+ "the sink expects " + outputExpr + " which its child does
not output: "
+ + rewrittenSink.child().getOutput());
+ }
+ // the constant column of the sink is refreshed to the fresh exprId of
the constant project
+ // constant propagation keeps the exprIds here, the sink still
references the output of its child
+ Assertions.assertEquals(studentId.getExprId(),
+ rewrittenSink.getOutputExprs().get(1).getExprId());
+ }
+
+ /**
+ * The aggregate still outputs the replaced column, so the constant
project item would shadow it and must be
+ * materialized with a fresh exprId.
+ */
+ private static void assertFreshConstantProjectItem(NamedExpression actual,
ExprId replacedExprId,
+ Expression expectedExpr, String expectedName) {
+ Alias alias = assertConstantProjectItem(actual, expectedExpr,
expectedName);
+ Assertions.assertNotEquals(replacedExprId, alias.getExprId(),
+ "the constant project item must not reuse the exprId of an
output which the aggregate still has");
+ }
+
+ /**
+ * The aggregate removed the constant grouping key from its outputs, so
the project item is the only definition
+ * of the column: its exprId is kept, rules and consumers such as the
materialized view matching rely on it.
+ */
+ private static void assertKeptConstantProjectItem(NamedExpression actual,
ExprId replacedExprId,
+ Expression expectedExpr, String expectedName) {
+ Alias alias = assertConstantProjectItem(actual, expectedExpr,
expectedName);
+ Assertions.assertEquals(replacedExprId, alias.getExprId(),
+ "the constant project item is the only definition, it must
keep the exprId of the column");
+ }
+
+ private static Alias assertConstantProjectItem(NamedExpression actual,
Expression expectedExpr,
+ String expectedName) {
+ Assertions.assertTrue(actual instanceof Alias, actual.toString());
+ Alias alias = (Alias) actual;
+ Assertions.assertEquals(expectedExpr, alias.child());
+ if (expectedName != null) {
+ Assertions.assertEquals(expectedName, alias.getName());
+ }
+ return alias;
}
@Test
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/CteConsumerOutputsRefresherTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/CteConsumerOutputsRefresherTest.java
new file mode 100644
index 00000000000..ae7f4cf55a2
--- /dev/null
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/CteConsumerOutputsRefresherTest.java
@@ -0,0 +1,107 @@
+// 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.
+
+package org.apache.doris.nereids.rules.rewrite;
+
+import org.apache.doris.nereids.trees.expressions.CTEId;
+import org.apache.doris.nereids.trees.expressions.ExprId;
+import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
+import org.apache.doris.nereids.trees.plans.RelationId;
+import org.apache.doris.nereids.trees.plans.logical.LogicalCTEConsumer;
+import org.apache.doris.nereids.types.IntegerType;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableMultimap;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * A CTE producer body is rewritten in its own subtree context, while the
consumers, which map to the producer
+ * outputs by exprId, were rewritten before it, so RewriteCteChildren
refreshes the two maps of the consumers with
+ * the exprIds the rules of the producer body published.
+ */
+public class CteConsumerOutputsRefresherTest {
+ private static final CTEId CTE_ID = new CTEId(0);
+ private static final ExprId PRODUCER_K = new ExprId(0);
+ private static final ExprId PRODUCER_V = new ExprId(1);
+ private static final ExprId CONSUMER_K = new ExprId(10);
+ private static final ExprId CONSUMER_V = new ExprId(11);
+
+ @Test
+ void testRefreshFollowsTheReplacementChain() {
+ // k is renamed twice, e.g. by two rules or by two passes of the same
rule
+ Map<ExprId, ExprId> remap = ImmutableMap.of(PRODUCER_K, new ExprId(2),
new ExprId(2), new ExprId(3));
+ Slot newProducerK = slot(new ExprId(3), "k");
+ LogicalCTEConsumer consumer = consumer(slot(PRODUCER_K, "k"),
slot(PRODUCER_V, "v"));
+
+ LogicalCTEConsumer refreshed = refresh(consumer, remap,
ImmutableList.of(newProducerK, slot(PRODUCER_V, "v")));
+ Assertions.assertEquals(newProducerK,
refreshed.getConsumerToProducerOutputMap().get(slot(CONSUMER_K, "k")));
+ Assertions.assertEquals(ImmutableList.of(slot(CONSUMER_K, "k")),
+
ImmutableList.copyOf(refreshed.getProducerToConsumerOutputMap().get(newProducerK)));
+ // v was not renamed, it keeps pointing to the same producer slot
+ Assertions.assertEquals(slot(PRODUCER_V, "v"),
+
refreshed.getConsumerToProducerOutputMap().get(slot(CONSUMER_V, "v")));
+ }
+
+ @Test
+ void testRenamedColumnWhichIsPrunedAfterwardsKeepsItsSlot() {
+ // the rule renamed k, but the rewrite of the producer body pruned the
column afterwards, so the producer
+ // does not output the new exprId any more and there is nothing to map
for it
+ Map<ExprId, ExprId> remap = ImmutableMap.of(PRODUCER_K, new ExprId(3));
+ Slot producerV = slot(PRODUCER_V, "v");
+ LogicalCTEConsumer consumer = consumer(slot(PRODUCER_K, "k"),
producerV);
+
+ LogicalCTEConsumer refreshed = refresh(consumer, remap,
ImmutableList.of(producerV));
+ Assertions.assertEquals(slot(PRODUCER_K, "k"),
+
refreshed.getConsumerToProducerOutputMap().get(slot(CONSUMER_K, "k")));
+ }
+
+ @Test
+ void testConsumerOfAnotherCteIsNotTouched() {
+ Slot producerK = slot(PRODUCER_K, "k");
+ LogicalCTEConsumer otherConsumer = new LogicalCTEConsumer(new
RelationId(1), new CTEId(1), "other",
+ ImmutableMap.of(slot(CONSUMER_K, "k"), producerK),
+ ImmutableMultimap.of(producerK, slot(CONSUMER_K, "k")));
+ LogicalCTEConsumer refreshed = (LogicalCTEConsumer)
otherConsumer.accept(
+ new RewriteCteChildren.CteConsumerOutputsRefresher(CTE_ID,
ImmutableMap.of(PRODUCER_K, new ExprId(3)),
+ ImmutableMap.of(new ExprId(3), slot(new ExprId(3),
"k"))), null);
+ Assertions.assertSame(otherConsumer, refreshed);
+ }
+
+ private static LogicalCTEConsumer refresh(LogicalCTEConsumer consumer,
Map<ExprId, ExprId> remap,
+ java.util.List<Slot> producerOutputs) {
+ Map<ExprId, Slot> producerOutputSlots = producerOutputs.stream()
+ .collect(Collectors.toMap(Slot::getExprId, slot -> slot));
+ return (LogicalCTEConsumer) consumer.accept(new
RewriteCteChildren.CteConsumerOutputsRefresher(
+ CTE_ID, remap, producerOutputSlots), null);
+ }
+
+ private static LogicalCTEConsumer consumer(Slot producerK, Slot producerV)
{
+ return new LogicalCTEConsumer(new RelationId(0), CTE_ID, "c",
+ ImmutableMap.of(slot(CONSUMER_K, "k"), producerK,
slot(CONSUMER_V, "v"), producerV),
+ ImmutableMultimap.of(producerK, slot(CONSUMER_K, "k"),
producerV, slot(CONSUMER_V, "v")));
+ }
+
+ private static Slot slot(ExprId exprId, String name) {
+ return new SlotReference(exprId, name, IntegerType.INSTANCE, true,
ImmutableList.of("test", "t"));
+ }
+}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ExprIdRewriterTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ExprIdRewriterTest.java
index 1acef3d6e74..148d8ecb648 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ExprIdRewriterTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/ExprIdRewriterTest.java
@@ -17,12 +17,23 @@
package org.apache.doris.nereids.rules.rewrite;
+import org.apache.doris.nereids.CascadesContext;
+import org.apache.doris.nereids.jobs.JobContext;
+import org.apache.doris.nereids.trees.expressions.Add;
import org.apache.doris.nereids.trees.expressions.Alias;
+import org.apache.doris.nereids.trees.expressions.EqualTo;
import org.apache.doris.nereids.trees.expressions.ExprId;
import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NamedExpression;
import org.apache.doris.nereids.trees.expressions.SlotReference;
import org.apache.doris.nereids.trees.expressions.literal.IntegerLiteral;
+import org.apache.doris.nereids.trees.plans.Plan;
+import org.apache.doris.nereids.trees.plans.commands.merge.MergeUtils;
+import org.apache.doris.nereids.trees.plans.logical.LogicalOlapScan;
+import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
import org.apache.doris.nereids.types.IntegerType;
+import org.apache.doris.nereids.util.MemoTestUtils;
+import org.apache.doris.nereids.util.PlanConstructor;
import com.google.common.collect.ImmutableList;
import org.junit.jupiter.api.Assertions;
@@ -30,6 +41,8 @@ import org.junit.jupiter.api.Test;
import java.util.HashMap;
import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
/**
* Test for ExprIdRewriter, especially for visitAlias functionality.
@@ -47,6 +60,62 @@ import java.util.Map;
*/
public class ExprIdRewriterTest {
+ @Test
+ void testGuardedBranchIsRewritten() {
+ // MERGE INTO wraps every assignment in a ShortCircuitIf, and such a
branch has to be rewritten as well:
+ // a reference which keeps an old exprId would point to a slot which
the rewritten child does not output
+ ExprId oldExprId = new ExprId(1);
+ ExprId newExprId = new ExprId(2);
+ SlotReference guardedSlot = slot(oldExprId, "k");
+ SlotReference conditionSlot = slot(new ExprId(3), "v");
+ // MergeUtils.selectBranch is what MergeIntoCommand builds every
assignment with
+ NamedExpression assignment = new Alias(new ExprId(9),
+ MergeUtils.selectBranch(new EqualTo(conditionSlot, new
IntegerLiteral(1)),
+ new Add(guardedSlot, new IntegerLiteral(1)),
guardedSlot), "value");
+
+ LogicalOlapScan scan = new
LogicalOlapScan(PlanConstructor.getNextRelationId(),
+ PlanConstructor.student, ImmutableList.of(""));
+ LogicalProject<LogicalOlapScan> project
+ = new LogicalProject<>(ImmutableList.of(assignment), scan);
+ Map<ExprId, ExprId> replaceMap = new HashMap<>();
+ replaceMap.put(oldExprId, newExprId);
+ CascadesContext cascadesContext =
MemoTestUtils.createCascadesContext(scan);
+ Plan rewritten = new ExprIdRewriter(new
ExprIdRewriter.ReplaceRule(replaceMap, true),
+ new JobContext(cascadesContext, null)).rewriteExpr(project,
replaceMap);
+
+ Expression rewrittenAssignment = ((Alias) ((LogicalProject<?>)
rewritten).getProjects().get(0)).child();
+ Set<ExprId> rewrittenSlotExprIds =
rewrittenAssignment.collect(expression -> expression instanceof SlotReference)
+ .stream().map(expression -> ((SlotReference)
expression).getExprId()).collect(Collectors.toSet());
+ Assertions.assertFalse(rewrittenSlotExprIds.contains(oldExprId),
rewrittenAssignment.toString());
+ Assertions.assertTrue(rewrittenSlotExprIds.contains(newExprId),
rewrittenAssignment.toString());
+ }
+
+ @Test
+ void testAliasIdIsRewrittenThroughThePlanRewrite() {
+ // a rule which renames an Alias (e.g. EliminateGroupByKeyByUniform)
publishes the alias exprId itself, and
+ // the plan rewrite has to replace it, not only the exprIds of the
slots below it
+ ExprId oldAliasExprId = new ExprId(8);
+ ExprId newAliasExprId = new ExprId(10);
+ NamedExpression item = new Alias(oldAliasExprId, new
IntegerLiteral(200), "field1");
+
+ LogicalOlapScan scan = new
LogicalOlapScan(PlanConstructor.getNextRelationId(),
+ PlanConstructor.student, ImmutableList.of(""));
+ LogicalProject<LogicalOlapScan> project = new
LogicalProject<>(ImmutableList.of(item), scan);
+ Map<ExprId, ExprId> replaceMap = new HashMap<>();
+ replaceMap.put(oldAliasExprId, newAliasExprId);
+ CascadesContext cascadesContext =
MemoTestUtils.createCascadesContext(scan);
+ Plan rewritten = new ExprIdRewriter(new
ExprIdRewriter.ReplaceRule(replaceMap, true),
+ new JobContext(cascadesContext, null)).rewriteExpr(project,
replaceMap);
+
+ NamedExpression rewrittenItem = ((LogicalProject<?>)
rewritten).getProjects().get(0);
+ Assertions.assertEquals(newAliasExprId, rewrittenItem.getExprId(),
rewrittenItem.toString());
+ Assertions.assertEquals("field1", rewrittenItem.getName());
+ }
+
+ private static SlotReference slot(ExprId exprId, String name) {
+ return new SlotReference(exprId, name, IntegerType.INSTANCE, true,
ImmutableList.of("test", "t"));
+ }
+
/**
* Test that visitAlias correctly rewrites Alias ExprId.
*
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
