Skip to content

Commit 611e5b5

Browse files
seawindeYour Name
authored andcommitted
[fix](nereids) Fix aggregate source repeat output is different from child repeat (#57840)
if cte def has group sets, after cte inline, the aggregate source repeat output is different from child repeat the pr fix this such query as following ```sql with t as ( select t1.l_partkey, t1.l_suppkey, o_orderdate, grouping(t1.l_suppkey), grouping(o_orderdate), grouping_id(t1.l_partkey, t1.l_suppkey), grouping_id(t1.l_partkey, t1.l_suppkey, o_orderdate), sum(o_totalprice), max(o_totalprice), min(o_totalprice), count(*), count(distinct case when o_shippriority > 1 and o_orderkey IN (1, 3) then o_custkey else null end) from (select * from lineitem where l_shipdate = '2023-12-11') t1 left join orders on t1.l_orderkey = orders.o_orderkey and t1.l_shipdate = o_orderdate group by GROUPING SETS ((l_shipdate, o_orderdate, l_partkey), (l_partkey, l_suppkey), (l_suppkey), ()) ) select t1.l_suppkey, t2.o_orderdate from t t1 inner join t t2 on t1.l_partkey = t2.l_partkey; ```
1 parent 43aaa7b commit 611e5b5

File tree

3 files changed

+89
-2
lines changed

3 files changed

+89
-2
lines changed

fe/fe-core/src/main/java/org/apache/doris/nereids/trees/copier/LogicalPlanDeepCopier.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,13 @@ public Plan visitLogicalAggregate(LogicalAggregate<? extends Plan> aggregate, De
186186
List<NamedExpression> outputExpressions = aggregate.getOutputExpressions().stream()
187187
.map(o -> (NamedExpression) ExpressionDeepCopier.INSTANCE.deepCopy(o, context))
188188
.collect(ImmutableList.toImmutableList());
189-
return aggregate.withChildGroupByAndOutput(groupByExpressions, outputExpressions, child);
189+
LogicalAggregate<Plan> copiedAggregate = aggregate.withChildGroupByAndOutput(groupByExpressions,
190+
outputExpressions, child);
191+
Optional<LogicalRepeat<? extends Plan>> childRepeat =
192+
copiedAggregate.collectFirst(LogicalRepeat.class::isInstance);
193+
return childRepeat.isPresent() ? aggregate.withChildGroupByAndOutputAndSourceRepeat(
194+
groupByExpressions, outputExpressions, child, childRepeat)
195+
: aggregate.withChildGroupByAndOutput(groupByExpressions, outputExpressions, child);
190196
}
191197

192198
@Override

fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalAggregate.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,13 @@ public LogicalAggregate<Plan> withChildGroupByAndOutput(List<Expression> groupBy
352352
hasPushed, withInProjection, sourceRepeat, Optional.empty(), Optional.empty(), newChild);
353353
}
354354

355+
public LogicalAggregate<Plan> withChildGroupByAndOutputAndSourceRepeat(List<Expression> groupByExprList,
356+
List<NamedExpression> outputExpressionList, Plan newChild,
357+
Optional<LogicalRepeat<?>> sourceRepeat) {
358+
return new LogicalAggregate<>(groupByExprList, outputExpressionList, normalized, ordinalIsResolved, generated,
359+
hasPushed, withInProjection, sourceRepeat, Optional.empty(), Optional.empty(), newChild);
360+
}
361+
355362
public LogicalAggregate<Plan> withChildAndOutput(CHILD_TYPE child,
356363
List<NamedExpression> outputExpressionList) {
357364
return new LogicalAggregate<>(groupByExpressions, outputExpressionList, normalized, ordinalIsResolved,

fe/fe-core/src/test/java/org/apache/doris/nereids/trees/copier/LogicalPlanDeepCopierTest.java

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,96 @@
1717

1818
package org.apache.doris.nereids.trees.copier;
1919

20+
import org.apache.doris.nereids.trees.expressions.Expression;
21+
import org.apache.doris.nereids.trees.expressions.NamedExpression;
2022
import org.apache.doris.nereids.trees.expressions.Slot;
23+
import org.apache.doris.nereids.trees.plans.Plan;
24+
import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
2125
import org.apache.doris.nereids.trees.plans.logical.LogicalOlapScan;
26+
import org.apache.doris.nereids.trees.plans.logical.LogicalRepeat;
2227
import org.apache.doris.nereids.util.PlanConstructor;
2328

29+
import com.google.common.collect.ImmutableList;
2430
import org.junit.jupiter.api.Assertions;
2531
import org.junit.jupiter.api.Test;
2632

33+
import java.util.List;
34+
import java.util.Optional;
35+
import java.util.stream.Collectors;
36+
2737
public class LogicalPlanDeepCopierTest {
2838

2939
@Test
3040
public void testDeepCopyOlapScan() {
3141
LogicalOlapScan relationPlan = PlanConstructor.newLogicalOlapScan(0, "a", 0);
3242
relationPlan = (LogicalOlapScan) relationPlan.withOperativeSlots(relationPlan.getOutput());
33-
LogicalOlapScan aCopy = (LogicalOlapScan) relationPlan.accept(LogicalPlanDeepCopier.INSTANCE, new DeepCopierContext());
43+
LogicalOlapScan aCopy =
44+
(LogicalOlapScan) relationPlan.accept(LogicalPlanDeepCopier.INSTANCE, new DeepCopierContext());
3445
for (Slot opSlot : aCopy.getOperativeSlots()) {
3546
Assertions.assertTrue(aCopy.getOutputSet().contains(opSlot));
3647
}
3748
}
49+
50+
@Test
51+
public void testDeepCopyAggregateWithSourceRepeat() {
52+
LogicalOlapScan scan = PlanConstructor.newLogicalOlapScan(0, "t", 0);
53+
List<? extends NamedExpression> groupingKeys = scan.getOutput().subList(0, 1);
54+
List<List<Expression>> groupingSets = ImmutableList.of(
55+
ImmutableList.of(groupingKeys.get(0)),
56+
ImmutableList.of()
57+
);
58+
LogicalRepeat<Plan> repeat = new LogicalRepeat<>(
59+
groupingSets,
60+
scan.getOutput().stream().map(NamedExpression.class::cast).collect(Collectors.toList()),
61+
scan
62+
);
63+
List<? extends NamedExpression> groupByExprs = repeat.getOutput().subList(0, 1).stream()
64+
.map(e -> (NamedExpression) e)
65+
.collect(ImmutableList.toImmutableList());
66+
List<? extends NamedExpression> outputExprs = repeat.getOutput();
67+
LogicalAggregate aggregate = new LogicalAggregate(
68+
groupByExprs,
69+
outputExprs,
70+
repeat
71+
);
72+
aggregate = aggregate.withSourceRepeat(repeat);
73+
DeepCopierContext context = new DeepCopierContext();
74+
LogicalAggregate<? extends Plan> copiedAggregate = (LogicalAggregate<? extends Plan>) aggregate.accept(
75+
LogicalPlanDeepCopier.INSTANCE,
76+
context
77+
);
78+
Assertions.assertTrue(copiedAggregate.getSourceRepeat().isPresent());
79+
80+
Optional<LogicalRepeat<? extends Plan>> copiedRepeat =
81+
copiedAggregate.collectFirst(LogicalRepeat.class::isInstance);
82+
Assertions.assertTrue(copiedRepeat.isPresent());
83+
Assertions.assertSame(copiedAggregate.getSourceRepeat().get(), copiedRepeat.get());
84+
85+
Assertions.assertNotSame(aggregate, copiedAggregate);
86+
Assertions.assertNotSame(repeat, copiedRepeat.get());
87+
}
88+
89+
@Test
90+
public void testDeepCopyAggregateWithoutSourceRepeat() {
91+
LogicalOlapScan scan = PlanConstructor.newLogicalOlapScan(0, "t", 0);
92+
List<Expression> groupByExprs = scan.getOutput().subList(0, 1).stream()
93+
.map(e -> (Expression) e)
94+
.collect(ImmutableList.toImmutableList());
95+
List<? extends NamedExpression> outputExprs = scan.getOutput();
96+
97+
LogicalAggregate aggregate = new LogicalAggregate(
98+
groupByExprs,
99+
outputExprs,
100+
scan
101+
);
102+
DeepCopierContext context = new DeepCopierContext();
103+
LogicalAggregate<? extends Plan> copiedAggregate = (LogicalAggregate<? extends Plan>) aggregate.accept(
104+
LogicalPlanDeepCopier.INSTANCE,
105+
context
106+
);
107+
Assertions.assertFalse(copiedAggregate.getSourceRepeat().isPresent());
108+
Assertions.assertNotSame(aggregate, copiedAggregate);
109+
Assertions.assertEquals(aggregate.getGroupByExpressions().size(),
110+
copiedAggregate.getGroupByExpressions().size());
111+
}
38112
}

0 commit comments

Comments
 (0)