This is an automated email from the ASF dual-hosted git repository. englefly pushed a commit to branch fix/4.2-cp-nullable-expr-id in repository https://gitbox.apache.org/repos/asf/doris.git
commit 429bfba5666270ab607d89727d63f0d14081f317 Author: minghong <[email protected]> AuthorDate: Fri Oct 30 00:03:36 2026 +0800 [fix](rbo) Align a materialized view plan with the query outputs before adding it to the memo ### What problem does this PR solve? Issue Number: None Problem Summary: The plan rewritten by a materialized view is copied into the memo of the main cost based optimizer as an alternative of the query plan, and Memo#copyIn requires the two plans to have the same root logical properties. The plan is skipped when a constant output keeps another exprId than the query plan, e.g. __bin_7#30 against __bin_7#33, so the view plan is silently dropped and the explain reports "... not chose" although the rewrite succeeded. Normalize the rewritten plan against the final origin plan first, and align the outputs when the logical properties still differ. The exprId of an output may only be replaced when both plans define it as the same constant, or when both describe the same column (name, type and, when both keep it, qualifier), otherwise the plan is refused: a same named column of another relation (e.g. a self join) must not be bound to the query exprId. ### Release note None ### Check List (For Author) - Test: Regression test (the mv suites) - Behavior changed: No - Does this need documentation: No --- .../org/apache/doris/nereids/CascadesContext.java | 12 ++- .../exploration/mv/MaterializedViewUtils.java | 88 ++++++++++++++++++++++ 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/CascadesContext.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/CascadesContext.java index 0215f007022..95d8d60574a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/CascadesContext.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/CascadesContext.java @@ -42,6 +42,7 @@ import org.apache.doris.nereids.properties.PhysicalProperties; import org.apache.doris.nereids.rules.RuleFactory; import org.apache.doris.nereids.rules.RuleSet; import org.apache.doris.nereids.rules.exploration.mv.MaterializationContext; +import org.apache.doris.nereids.rules.exploration.mv.MaterializedViewUtils; import org.apache.doris.nereids.trees.expressions.Alias; import org.apache.doris.nereids.trees.expressions.CTEId; import org.apache.doris.nereids.trees.expressions.Expression; @@ -276,15 +277,22 @@ public class CascadesContext implements ScheduleContext { if (!statementContext.getRewrittenPlansByMv().isEmpty()) { // copy tmp plan for mv rewrite firstly for (Plan rewrittenPlan : rewrittenPlansByMv) { + // the rewritten plan may define a constant output with another exprId than the origin plan, align + // the outputs before the comparison, only a same constant is allowed to change the exprId + Plan alignedPlan = MaterializedViewUtils.normalizeSinkExpressions(rewrittenPlan, plan); + if (alignedPlan != null && !alignedPlan.getLogicalProperties().equals(plan.getLogicalProperties())) { + alignedPlan = MaterializedViewUtils.alignPlanOutputWithOrigin(rewrittenPlan, plan); + } // aggregate_without_roll_up query_13_0 cause error into targetGroup but differ in logical properties // tmp rewritten plan output is different from final rewritten plan output - if (!rewrittenPlan.getLogicalProperties().equals(plan.getLogicalProperties())) { + if (alignedPlan == null + || !alignedPlan.getLogicalProperties().equals(plan.getLogicalProperties())) { LOG.error("rewritten plan in rbo logical properties are " + "different from original plan, query id is {}", getConnectContext().getQueryIdentifier()); continue; } - this.memo.copyIn(rewrittenPlan, this.memo.getRoot(), false); + this.memo.copyIn(alignedPlan, this.memo.getRoot(), false); } } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializedViewUtils.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializedViewUtils.java index 71d36282c34..0fec5373c55 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializedViewUtils.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/MaterializedViewUtils.java @@ -479,6 +479,94 @@ public class MaterializedViewUtils { } + /** + * Align the outputs of a plan rewritten by a materialized view with the origin query plan, so that both plans + * can be alternatives of one memo group. + * + * <p>The exprId of an output may only be replaced when both plans define that output as the same constant: + * a column which a predicate pins to a literal can be materialized with a fresh exprId, and the two plans then + * still compute the same value. For every other output the exprId has to be the same already, otherwise the two + * plans are not known to produce the same columns (e.g. a self join can expose two same named columns of two + * relations) and the rewritten plan must not be used. + */ + public static Plan alignPlanOutputWithOrigin(Plan rewrittenPlan, Plan originPlan) { + List<Slot> originOutputs = originPlan.getOutput(); + List<Slot> rewrittenOutputs = rewrittenPlan instanceof LogicalSink + ? ((LogicalSink<?>) rewrittenPlan).child().getOutput() + : rewrittenPlan.getOutput(); + if (originOutputs.size() != rewrittenOutputs.size()) { + return null; + } + Map<ExprId, Expression> originOutputDefinitions = collectNamedExpressionDefinitions(originPlan); + List<NamedExpression> alignedOutputs = new ArrayList<>(originOutputs.size()); + for (int i = 0; i < originOutputs.size(); i++) { + Slot originSlot = originOutputs.get(i); + Slot rewrittenSlot = rewrittenOutputs.get(i); + if (rewrittenSlot.getExprId().equals(originSlot.getExprId())) { + alignedOutputs.add(rewrittenSlot); + continue; + } + if (!isSameConstant(rewrittenSlot, originOutputDefinitions.get(originSlot.getExprId())) + && !isSameColumn(rewrittenSlot, originSlot)) { + return null; + } + Expression inner = rewrittenSlot.nullable() == originSlot.nullable() + ? rewrittenSlot + : (originSlot.nullable() ? new Nullable(rewrittenSlot) : new NonNullable(rewrittenSlot)); + alignedOutputs.add(new Alias(originSlot.getExprId(), inner, originSlot.getName())); + } + if (rewrittenPlan instanceof LogicalSink) { + LogicalSink<?> sink = (LogicalSink<?>) rewrittenPlan; + LogicalProject<Plan> project = new LogicalProject<>(alignedOutputs, sink.child()); + List<NamedExpression> sinkOutputs = alignedOutputs.stream() + .map(output -> (NamedExpression) output.toSlot()) + .collect(ImmutableList.toImmutableList()); + return sink.withOutputExprs(sinkOutputs).withChildren(project); + } + return new LogicalProject<>(alignedOutputs, rewrittenPlan); + } + + /** + * Whether the rewritten output describes the same column as the origin one. The rewritten plan is built by the + * materialized view rewrite which keeps the position of the query outputs, so the name, the type and (when both + * sides keep it) the qualifier have to match. This rejects a same named column of another relation, e.g. a self + * join, and an output which is added or removed and shifts the columns. + */ + private static boolean isSameColumn(Slot rewrittenSlot, Slot originSlot) { + if (!rewrittenSlot.getName().equalsIgnoreCase(originSlot.getName()) + || !rewrittenSlot.getDataType().equals(originSlot.getDataType())) { + return false; + } + List<String> rewrittenQualifier = rewrittenSlot.getQualifier(); + List<String> originQualifier = originSlot.getQualifier(); + return rewrittenQualifier.isEmpty() || originQualifier.isEmpty() + || rewrittenQualifier.equals(originQualifier); + } + + private static Map<ExprId, Expression> collectNamedExpressionDefinitions(Plan plan) { + Map<ExprId, Expression> definitions = Maps.newHashMap(); + plan.foreach(node -> { + for (Expression expression : ((Plan) node).getExpressions()) { + if (expression instanceof NamedExpression) { + definitions.putIfAbsent(((NamedExpression) expression).getExprId(), expression); + } + } + return false; + }); + return definitions; + } + + private static boolean isSameConstant(NamedExpression rewrittenExpr, Expression originDefinition) { + if (originDefinition == null || !originDefinition.isConstant() || !rewrittenExpr.isConstant()) { + return false; + } + Expression rewrittenConstant = rewrittenExpr instanceof Alias ? rewrittenExpr.child(0) : rewrittenExpr; + Expression originConstant = originDefinition instanceof Alias + ? originDefinition.child(0) : originDefinition; + return rewrittenConstant.getDataType().equals(originConstant.getDataType()) + && rewrittenConstant.equals(originConstant); + } + /** * Normalize expression with query, keep the consistency of exprId and nullable props with * query --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
