From 7d490f324e066b5b21a3a7d625ab1454ec3dcfbb Mon Sep 17 00:00:00 2001 From: seawinde Date: Fri, 18 Jul 2025 14:26:07 +0800 Subject: [PATCH] [fix](nereids) Fix the expr id are same but different expr when agg table with random distribute (#52993) If agg table is random hash distribute, would add aggregate node on scan. The aggregate function alias expr id is same to the child expr id of alias. such as query sql is `select * from db1.tagg` the query plan is as following, and the `sum(b#1) AS `b`#1`, alias expr id is same to the child expr id of alias, the id is 1 this would cause hidden problems ```sql LogicalResultSink[30] ( outputExprs=[a#0, b#1] ) +--LogicalAggregate[29] ( groupByExpr=[a#0], outputExpr=[a#0, sum(b#1) AS `b`#1], hasRepeat=false, stats=1 ) +--LogicalOlapScan ( qualified=internal.db1.tagg, indexName=, selectedIndexId=1752042452160, preAgg=ON, operativeCol=[a#0, b#1], stats=1 ) ``` the pr fix this, and the expression change to `sum(b#1) AS `b`#4` ```sql LogicalResultSink[30] ( outputExprs=[a#0, b#4] ) +--LogicalAggregate[29] ( groupByExpr=[a#0], outputExpr=[a#0, sum(b#1) AS `b`#4], hasRepeat=false, stats=1 ) +--LogicalOlapScan ( qualified=internal.db1.tagg, indexName=, selectedIndexId=1752042065062, preAgg=ON, operativeCol=[a#0, b#1], stats=1 ) ``` --- .../nereids/rules/analysis/BindRelation.java | 4 +- .../rules/analysis/BindRelationTest.java | 43 +++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindRelation.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindRelation.java index cbc2a93f18bad4..0e07fa08f97f1f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindRelation.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindRelation.java @@ -282,8 +282,8 @@ private LogicalPlan preAggForRandomDistribution(LogicalOlapScan olapScan) { if (function == null) { return olapScan; } - Alias alias = new Alias(exprId, ImmutableList.of(function), col.getName(), - olapScan.qualified(), true); + Alias alias = new Alias(StatementScopeIdGenerator.newExprId(), ImmutableList.of(function), + col.getName(), olapScan.qualified(), true); outputExpressions.add(alias); } } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/BindRelationTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/BindRelationTest.java index eaeaa3b2edda8b..428254a91c472a 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/BindRelationTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/BindRelationTest.java @@ -20,10 +20,16 @@ import org.apache.doris.nereids.analyzer.UnboundRelation; import org.apache.doris.nereids.pattern.GeneratedPlanPatterns; import org.apache.doris.nereids.rules.RulePromise; +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.NamedExpression; import org.apache.doris.nereids.trees.expressions.StatementScopeIdGenerator; 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.LogicalOlapScan; +import org.apache.doris.nereids.trees.plans.visitor.DefaultPlanVisitor; +import org.apache.doris.nereids.util.PlanChecker; import org.apache.doris.nereids.util.PlanRewriter; import org.apache.doris.utframe.TestWithFeService; @@ -31,6 +37,11 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + class BindRelationTest extends TestWithFeService implements GeneratedPlanPatterns { private static final String DB1 = "db1"; private static final String DB2 = "db2"; @@ -93,6 +104,38 @@ void bindRandomAggTable() { plan.getOutput().get(1).getQualifier()); } + @Test + void testBindRandomAggTableExprIdSame() { + connectContext.getSessionVariable().setDisableNereidsRules("PRUNE_EMPTY_PARTITION"); + connectContext.getState().setIsQuery(true); + PlanChecker.from(connectContext) + .checkPlannerResult("select * from db1.tagg", + planner -> { + List collectedAlias = new ArrayList<>(); + planner.getCascadesContext().getRewritePlan().accept( + new DefaultPlanVisitor>() { + @Override + public Void visitLogicalAggregate(LogicalAggregate aggregate, + List context) { + for (Expression expression : aggregate.getExpressions()) { + collectedAlias.addAll( + expression.collectToList(Alias.class::isInstance)); + } + return super.visitLogicalAggregate(aggregate, context); + } + }, collectedAlias); + for (Alias alias : collectedAlias) { + for (Expression child : alias.children()) { + Set childExpressionSet = + child.collectToSet(NamedExpression.class::isInstance).stream() + .map(expr -> ((NamedExpression) expr).getExprId()) + .collect(Collectors.toSet()); + Assertions.assertFalse(childExpressionSet.contains(alias.getExprId())); + } + } + }); + } + @Override public RulePromise defaultPromise() { return RulePromise.REWRITE;