This is an automated email from the ASF dual-hosted git repository.
morrySnow 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 e513aafab2a [fix](repeat) Preserve grouping ID after repeat
decomposition (#65968)
e513aafab2a is described below
commit e513aafab2a0915d3807bab5002961daf1ba45d9
Author: feiniaofeiafei <[email protected]>
AuthorDate: Tue Aug 4 10:38:13 2026 +0800
[fix](repeat) Preserve grouping ID after repeat decomposition (#65968)
### What problem does this PR solve?
Related PR: #59116
`DecomposeRepeatWithPreAggregation` dropped the internal `GROUPING_ID`
column and recalculated IDs after removing the largest grouping set.
This breaks upper plans that reference `GROUPING_ID`, and changes the
original grouping-ID semantics.
This PR preserves original grouping-ID values for the residual Repeat,
appends the removed grouping set’s original ID to the direct branch, and
keeps the internal slot through the rewritten `Project` and `Union`.
### Release note
Fix internal `GROUPING_ID` handling after Repeat decomposition.
---
.../nereids/rules/analysis/NormalizeRepeat.java | 2 +-
.../mv/AbstractMaterializedViewAggregateRule.java | 1 +
.../LogicalRepeatToPhysicalRepeat.java | 1 +
.../rewrite/DecomposeRepeatWithPreAggregation.java | 49 ++++++++------
.../trees/copier/LogicalPlanDeepCopier.java | 4 ++
.../doris/nereids/trees/plans/algebra/Repeat.java | 14 +++-
.../nereids/trees/plans/logical/LogicalRepeat.java | 74 +++++++++++++++++-----
.../trees/plans/physical/PhysicalRepeat.java | 48 +++++++++++---
.../DecomposeRepeatWithPreAggregationTest.java | 60 ++++++++++++++++--
.../nereids/trees/plans/algebra/RepeatTest.java | 20 ++++++
10 files changed, 218 insertions(+), 55 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeRepeat.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeRepeat.java
index bbabf96f7f1..b8381655d81 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeRepeat.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeRepeat.java
@@ -214,7 +214,7 @@ public class NormalizeRepeat extends OneAnalysisRuleFactory
{
normalizedRepeatOutput = normalizedRepeatOutput.stream()
.filter(expr -> !expr.equals(groupingId))
.collect(Collectors.toList());
- LogicalRepeat<Plan> normalizedRepeat = repeat.withNormalizedExpr(
+ LogicalRepeat<Plan> normalizedRepeat = repeat.withGroupingIdValues(
(List) normalizedGroupingSets, normalizedRepeatOutput,
groupingId, normalizedChild);
List<Expression> normalizedAggGroupBy =
ImmutableList.<Expression>builder()
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java
index b9c6e2579ab..7ee28753abc 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/mv/AbstractMaterializedViewAggregateRule.java
@@ -261,6 +261,7 @@ public abstract class AbstractMaterializedViewAggregateRule
extends AbstractMate
}
LogicalRepeat<Plan> repeat = new
LogicalRepeat<>(rewrittenGroupSetsExpressions,
finalOutputExpressions,
queryStructInfo.getGroupingId().get(),
+
queryAggregate.getSourceRepeat().get().getGroupingIdValues().orElse(null),
queryAggregate.getSourceRepeat().get().getRepeatType(),
tempRewritedPlan);
return NormalizeRepeat.doNormalize(repeat);
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalRepeatToPhysicalRepeat.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalRepeatToPhysicalRepeat.java
index 00d89034327..4f25d960dd6 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalRepeatToPhysicalRepeat.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalRepeatToPhysicalRepeat.java
@@ -32,6 +32,7 @@ public class LogicalRepeatToPhysicalRepeat extends
OneImplementationRuleFactory
repeat.getGroupingSets(),
repeat.getOutputExpressions(),
repeat.getGroupingId().get(),
+ repeat.getGroupingIdValues(),
repeat.getLogicalProperties(),
repeat.child()
)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/DecomposeRepeatWithPreAggregation.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/DecomposeRepeatWithPreAggregation.java
index e83160ce06c..e0f81b2cf36 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/DecomposeRepeatWithPreAggregation.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/DecomposeRepeatWithPreAggregation.java
@@ -151,45 +151,47 @@ public class DecomposeRepeatWithPreAggregation extends
DefaultPlanRewriter<Disti
LogicalRepeat<Plan> repeat = (LogicalRepeat<Plan>) aggregate.child();
List<List<Expression>> newGroupingSets = new ArrayList<>();
+ List<List<Long>> groupingFunctionsValues =
repeat.computeGroupingFunctionsValues();
+ List<Long> originalGroupingIdValues = groupingFunctionsValues.get(0);
+ List<Long> remainingGroupingIdValues = new ArrayList<>();
for (int i = 0; i < repeat.getGroupingSets().size(); ++i) {
if (i == maxGroupIndex) {
continue;
}
newGroupingSets.add(repeat.getGroupingSets().get(i));
+ remainingGroupingIdValues.add(originalGroupingIdValues.get(i));
}
List<NamedExpression> groupingFunctionSlots = new ArrayList<>();
LogicalRepeat<Plan> newRepeat = constructRepeat(repeat,
aggregateConsumer, newGroupingSets,
- originToConsumerMap, groupingFunctionSlots);
+ remainingGroupingIdValues, originToConsumerMap,
groupingFunctionSlots);
Set<Expression> needRemovedExprSet = getNeedAddNullExpressions(repeat,
newGroupingSets, maxGroupIndex);
Map<AggregateFunction, Slot> aggFuncToSlot = new HashMap<>();
LogicalAggregate<Plan> topAgg = constructAgg(aggregate,
originToConsumerMap, newRepeat, groupingFunctionSlots,
aggFuncToSlot);
LogicalProject<Plan> project = constructProject(aggregate,
originToConsumerMap, needRemovedExprSet,
- groupingFunctionSlots, topAgg, aggFuncToSlot);
- LogicalPlan directChild = getDirectChild(directConsumer,
groupingFunctionSlots);
+ groupingFunctionSlots, newRepeat.getGroupingId().get(),
topAgg, aggFuncToSlot);
+ LogicalPlan directChild = getDirectChild(directConsumer,
groupingFunctionsValues, maxGroupIndex);
return constructUnion(project, directChild, aggregate);
}
/**
* Get the direct child plan for the union operation.
- * If there are grouping function slots, wrap the consumer with a project
that adds
- * zero literals for each grouping function slot to match the output
schema.
+ * If the output contains internal grouping id or grouping function slots,
wrap the consumer with a project
+ * that adds the values of the internal grouping id and grouping scalar
functions for the maximum grouping set.
*
* @param directConsumer the CTE consumer for the direct path
- * @param groupingFunctionSlots the list of grouping function slots to
handle
+ * @param groupingFunctionsValues internal grouping id and grouping scalar
function values for all grouping sets
+ * @param maxGroupIndex index of the maximum grouping set
* @return the direct child plan, possibly wrapped with a project
*/
- private LogicalPlan getDirectChild(LogicalCTEConsumer directConsumer,
List<NamedExpression> groupingFunctionSlots) {
- LogicalPlan directChild = directConsumer;
- if (!groupingFunctionSlots.isEmpty()) {
- ImmutableList.Builder<NamedExpression> builder =
ImmutableList.builder();
- builder.addAll(directConsumer.getOutput());
- for (int i = 0; i < groupingFunctionSlots.size(); ++i) {
- builder.add(new Alias(new BigIntLiteral(0)));
- }
- directChild = new LogicalProject<Plan>(builder.build(),
directConsumer);
- }
- return directChild;
+ private LogicalPlan getDirectChild(LogicalCTEConsumer directConsumer,
+ List<List<Long>> groupingFunctionsValues, int maxGroupIndex) {
+ ImmutableList.Builder<NamedExpression> builder =
ImmutableList.builder();
+ builder.addAll(directConsumer.getOutput());
+ for (List<Long> values : groupingFunctionsValues) {
+ builder.add(new Alias(new
BigIntLiteral(values.get(maxGroupIndex))));
+ }
+ return new LogicalProject<Plan>(builder.build(), directConsumer);
}
/**
@@ -304,7 +306,7 @@ public class DecomposeRepeatWithPreAggregation extends
DefaultPlanRewriter<Disti
*/
private LogicalProject<Plan> constructProject(LogicalAggregate<? extends
Plan> aggregate,
Map<Slot, Slot> originToConsumerMap, Set<Expression>
needRemovedExprSet,
- List<NamedExpression> groupingFunctionSlots,
LogicalAggregate<Plan> topAgg,
+ List<NamedExpression> groupingFunctionSlots, SlotReference
groupingId, LogicalAggregate<Plan> topAgg,
Map<AggregateFunction, Slot> aggFuncToSlot) {
LogicalRepeat<?> repeat = (LogicalRepeat<?>) aggregate.child(0);
Set<ExprId> originGroupingFunctionId = new HashSet<>();
@@ -335,6 +337,7 @@ public class DecomposeRepeatWithPreAggregation extends
DefaultPlanRewriter<Disti
projects.add(replacedExpr.toSlot());
}
}
+ projects.add(groupingId);
projects.addAll(groupingFunctionSlots);
return new LogicalProject<>(projects.build(), topAgg);
}
@@ -370,6 +373,7 @@ public class DecomposeRepeatWithPreAggregation extends
DefaultPlanRewriter<Disti
}
unionOutputs.add(expr.toSlot());
}
+ unionOutputs.add(repeat.getGroupingId().get());
unionOutputs.addAll(groupingFunctionSlots);
return new LogicalUnion(Qualifier.ALL, unionOutputs, childrenOutputs,
ImmutableList.of(),
false, ImmutableList.of(aggregateProject, directConsumer));
@@ -601,7 +605,8 @@ public class DecomposeRepeatWithPreAggregation extends
DefaultPlanRewriter<Disti
* @return a new LogicalRepeat with replaced expressions
*/
private LogicalRepeat<Plan> constructRepeat(LogicalRepeat<Plan> repeat,
LogicalPlan child,
- List<List<Expression>> newGroupingSets, Map<Slot, Slot>
producerToDirectConsumerSlotMap,
+ List<List<Expression>> newGroupingSets, List<Long>
remainingGroupingIdValues,
+ Map<Slot, Slot> producerToDirectConsumerSlotMap,
List<NamedExpression> groupingFunctionSlots) {
List<List<Expression>> replacedNewGroupingSets = new ArrayList<>();
for (List<Expression> groupingSet : newGroupingSets) {
@@ -617,7 +622,9 @@ public class DecomposeRepeatWithPreAggregation extends
DefaultPlanRewriter<Disti
for (NamedExpression groupingFunction : newGroupingFunctions) {
groupingFunctionSlots.add(groupingFunction.toSlot());
}
- return repeat.withNormalizedExpr(replacedNewGroupingSets,
replacedRepeatOutputs,
- repeat.getGroupingId().get(), child);
+ Slot groupingId = repeat.getGroupingId().get();
+ return repeat.withGroupingIdValues(replacedNewGroupingSets,
replacedRepeatOutputs,
+ new SlotReference(groupingId.getName(),
groupingId.getDataType(), false), remainingGroupingIdValues,
+ child);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/copier/LogicalPlanDeepCopier.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/copier/LogicalPlanDeepCopier.java
index 55d6497eae1..29fd9e9e4c5 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/copier/LogicalPlanDeepCopier.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/copier/LogicalPlanDeepCopier.java
@@ -217,6 +217,10 @@ public class LogicalPlanDeepCopier extends
DefaultPlanRewriter<DeepCopierContext
.collect(ImmutableList.toImmutableList());
SlotReference groupingId = (SlotReference)
ExpressionDeepCopier.INSTANCE
.deepCopy(repeat.getGroupingId().get(), context);
+ if (repeat.getGroupingIdValues().isPresent()) {
+ return new LogicalRepeat<>(groupingSets, outputExpressions,
groupingId,
+ repeat.getGroupingIdValues().get(),
repeat.getRepeatType(), child);
+ }
return new LogicalRepeat<>(groupingSets, outputExpressions,
groupingId, repeat.getRepeatType(), child);
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/Repeat.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/Repeat.java
index 2e0dde6c305..f3f490da6d4 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/Repeat.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/Repeat.java
@@ -35,6 +35,7 @@ import org.apache.commons.lang3.StringUtils;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
@@ -49,6 +50,17 @@ public interface Repeat<CHILD_PLAN extends Plan> extends
Aggregate<CHILD_PLAN> {
List<NamedExpression> getOutputExpressions();
+ /**
+ * Values to fill the internal GROUPING_ID slot for each grouping set.
+ *
+ * <p>Most repeat nodes derive these values from their grouping sets. A
repeat split by
+ * {@code DecomposeRepeatWithPreAggregation} keeps the values from the
original repeat so
+ * that its GROUPING_ID remains stable after one grouping set is
removed.</p>
+ */
+ default Optional<List<Long>> getGroupingIdValues() {
+ return Optional.empty();
+ }
+
@Override
default List<Expression> getGroupByExpressions() {
return
ImmutableList.copyOf(ExpressionUtils.flatExpressions(getGroupingSets()));
@@ -107,7 +119,7 @@ public interface Repeat<CHILD_PLAN extends Plan> extends
Aggregate<CHILD_PLAN> {
List<GroupingScalarFunction> functions = ExpressionUtils.collectToList(
getOutputExpressions(),
GroupingScalarFunction.class::isInstance);
List<List<Long>> groupingFunctionsValues = Lists.newArrayList();
- groupingFunctionsValues.add(shapes.computeGroupingIdValue());
+
groupingFunctionsValues.add(getGroupingIdValues().orElseGet(shapes::computeGroupingIdValue));
for (GroupingScalarFunction function : functions) {
groupingFunctionsValues.add(function.computeValue(shapes));
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalRepeat.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalRepeat.java
index 544255a68a2..7f3567eeaa9 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalRepeat.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalRepeat.java
@@ -55,6 +55,7 @@ public class LogicalRepeat<CHILD_TYPE extends Plan> extends
LogicalUnary<CHILD_T
public static final int MAX_GROUPING_SETS_NUM = 64;
private final List<List<Expression>> groupingSets;
+ private final Optional<List<Long>> groupingIdValues;
private final List<NamedExpression> outputExpressions;
private final Optional<SlotReference> groupingId;
private final boolean withInProjection;
@@ -81,7 +82,22 @@ public class LogicalRepeat<CHILD_TYPE extends Plan> extends
LogicalUnary<CHILD_T
RepeatType type,
CHILD_TYPE child) {
this(groupingSets, outputExpressions, Optional.empty(),
Optional.empty(),
- Optional.ofNullable(groupingId), true, type, child);
+ Optional.ofNullable(groupingId), Optional.empty(), true, type,
child);
+ }
+
+ /**
+ * Desc: Constructor for LogicalRepeat with precomputed internal grouping
id values.
+ */
+ public LogicalRepeat(
+ List<List<Expression>> groupingSets,
+ List<NamedExpression> outputExpressions,
+ SlotReference groupingId,
+ List<Long> groupingIdValues,
+ RepeatType type,
+ CHILD_TYPE child) {
+ this(groupingSets, outputExpressions, Optional.empty(),
Optional.empty(),
+ Optional.ofNullable(groupingId),
Optional.ofNullable(groupingIdValues), true,
+ type, child);
}
/**
@@ -93,7 +109,8 @@ public class LogicalRepeat<CHILD_TYPE extends Plan> extends
LogicalUnary<CHILD_T
Optional<SlotReference> groupingId,
RepeatType type,
CHILD_TYPE child) {
- this(groupingSets, outputExpressions, Optional.empty(),
Optional.empty(), groupingId, true, type, child);
+ this(groupingSets, outputExpressions, Optional.empty(),
Optional.empty(), groupingId, Optional.empty(),
+ true, type, child);
}
/**
@@ -101,7 +118,8 @@ public class LogicalRepeat<CHILD_TYPE extends Plan> extends
LogicalUnary<CHILD_T
*/
private LogicalRepeat(List<List<Expression>> groupingSets,
List<NamedExpression> outputExpressions,
Optional<GroupExpression> groupExpression,
Optional<LogicalProperties> logicalProperties,
- Optional<SlotReference> groupingId, boolean withInProjection,
RepeatType type, CHILD_TYPE child) {
+ Optional<SlotReference> groupingId, Optional<List<Long>>
groupingIdValues,
+ boolean withInProjection, RepeatType type, CHILD_TYPE child) {
super(PlanType.LOGICAL_REPEAT, groupExpression, logicalProperties,
child);
this.groupingSets = Objects.requireNonNull(groupingSets, "groupingSets
can not be null")
.stream()
@@ -110,6 +128,10 @@ public class LogicalRepeat<CHILD_TYPE extends Plan>
extends LogicalUnary<CHILD_T
this.outputExpressions = ImmutableList.copyOf(
Objects.requireNonNull(outputExpressions, "outputExpressions
can not be null"));
this.groupingId = groupingId;
+ this.groupingIdValues = groupingIdValues.map(ImmutableList::copyOf);
+ Preconditions.checkArgument(!this.groupingIdValues.isPresent()
+ || this.groupingIdValues.get().size() ==
this.groupingSets.size(),
+ "groupingIdValues size is different from groupingSets size");
this.withInProjection = withInProjection;
this.type = type;
}
@@ -128,6 +150,11 @@ public class LogicalRepeat<CHILD_TYPE extends Plan>
extends LogicalUnary<CHILD_T
return groupingId;
}
+ @Override
+ public Optional<List<Long>> getGroupingIdValues() {
+ return groupingIdValues;
+ }
+
public RepeatType getRepeatType() {
return type;
}
@@ -155,7 +182,8 @@ public class LogicalRepeat<CHILD_TYPE extends Plan> extends
LogicalUnary<CHILD_T
return Utils.toSqlString("LogicalRepeat",
"groupingSets", groupingSets,
"outputExpressions", outputExpressions,
- "groupingId", groupingId
+ "groupingId", groupingId,
+ "groupingIdValues", groupingIdValues
);
}
@@ -216,26 +244,28 @@ public class LogicalRepeat<CHILD_TYPE extends Plan>
extends LogicalUnary<CHILD_T
}
LogicalRepeat<?> that = (LogicalRepeat<?>) o;
return Objects.equals(groupingSets, that.groupingSets) &&
Objects.equals(outputExpressions,
- that.outputExpressions) && Objects.equals(groupingId,
that.groupingId);
+ that.outputExpressions) && Objects.equals(groupingId,
that.groupingId)
+ && Objects.equals(groupingIdValues, that.groupingIdValues);
}
@Override
public int hashCode() {
- return Objects.hash(groupingSets, outputExpressions, groupingId);
+ return Objects.hash(groupingSets, outputExpressions, groupingId,
groupingIdValues);
}
@Override
public LogicalRepeat<Plan> withChildren(List<Plan> children) {
Preconditions.checkArgument(children.size() == 1);
return AbstractPlan.copyWithSameId(this, () ->
- new LogicalRepeat<>(groupingSets, outputExpressions,
groupingId, type, children.get(0)));
+ new LogicalRepeat<>(groupingSets, outputExpressions,
Optional.empty(), Optional.empty(),
+ groupingId, groupingIdValues, true, type,
children.get(0)));
}
@Override
public LogicalRepeat<CHILD_TYPE>
withGroupExpression(Optional<GroupExpression> groupExpression) {
return AbstractPlan.copyWithSameId(this, () ->
new LogicalRepeat<>(groupingSets, outputExpressions,
groupExpression,
- Optional.of(getLogicalProperties()), groupingId,
withInProjection, type, child()));
+ Optional.of(getLogicalProperties()), groupingId,
groupingIdValues, withInProjection, type, child()));
}
@Override
@@ -244,41 +274,51 @@ public class LogicalRepeat<CHILD_TYPE extends Plan>
extends LogicalUnary<CHILD_T
Preconditions.checkArgument(children.size() == 1);
return AbstractPlan.copyWithSameId(this, () ->
new LogicalRepeat<>(groupingSets, outputExpressions,
groupExpression, logicalProperties,
- groupingId, withInProjection, type, children.get(0)));
+ groupingId, groupingIdValues, withInProjection, type,
children.get(0)));
}
public LogicalRepeat<CHILD_TYPE> withGroupSets(List<List<Expression>>
groupingSets) {
return AbstractPlan.copyWithSameId(this, () ->
- new LogicalRepeat<>(groupingSets, outputExpressions,
groupingId, type, child()));
+ new LogicalRepeat<>(groupingSets, outputExpressions,
Optional.empty(), Optional.empty(), groupingId,
+ groupingIdValues.filter(values -> values.size() ==
groupingSets.size()), true, type, child()));
}
public LogicalRepeat<CHILD_TYPE>
withGroupSetsAndOutput(List<List<Expression>> groupingSets,
List<NamedExpression> outputExpressionList) {
return AbstractPlan.copyWithSameId(this, () ->
- new LogicalRepeat<>(groupingSets, outputExpressionList,
groupingId, type, child()));
+ new LogicalRepeat<>(groupingSets, outputExpressionList,
Optional.empty(), Optional.empty(), groupingId,
+ groupingIdValues.filter(values -> values.size() ==
groupingSets.size()), true, type, child()));
}
@Override
public LogicalRepeat<CHILD_TYPE> withAggOutput(List<NamedExpression>
newOutput) {
return AbstractPlan.copyWithSameId(this, () ->
- new LogicalRepeat<>(groupingSets, newOutput, groupingId, type,
child()));
+ new LogicalRepeat<>(groupingSets, newOutput, Optional.empty(),
Optional.empty(), groupingId,
+ groupingIdValues, true, type, child()));
}
- public LogicalRepeat<Plan> withNormalizedExpr(List<List<Expression>>
groupingSets,
+ public LogicalRepeat<Plan> withGroupingIdValues(List<List<Expression>>
groupingSets,
List<NamedExpression> outputExpressionList, SlotReference
groupingId, Plan child) {
return AbstractPlan.copyWithSameId(this, () ->
- new LogicalRepeat<>(groupingSets, outputExpressionList,
groupingId, type, child));
+ new LogicalRepeat<>(groupingSets, outputExpressionList,
Optional.empty(), Optional.empty(),
+ Optional.ofNullable(groupingId),
+ groupingIdValues.filter(values -> values.size() ==
groupingSets.size()), true, type, child));
}
- public LogicalRepeat<Plan> withAggOutputAndChild(List<NamedExpression>
newOutput, Plan child) {
+ /**
+ * Create a normalized repeat that keeps the supplied original internal
grouping id values.
+ */
+ public LogicalRepeat<Plan> withGroupingIdValues(List<List<Expression>>
groupingSets,
+ List<NamedExpression> outputExpressionList, SlotReference
groupingId, List<Long> groupingIdValues,
+ Plan child) {
return AbstractPlan.copyWithSameId(this, () ->
- new LogicalRepeat<>(groupingSets, newOutput, groupingId, type,
child));
+ new LogicalRepeat<>(groupingSets, outputExpressionList,
groupingId, groupingIdValues, type, child));
}
public LogicalRepeat<CHILD_TYPE> withInProjection(boolean
withInProjection) {
return AbstractPlan.copyWithSameId(this, () ->
new LogicalRepeat<>(groupingSets, outputExpressions,
- Optional.empty(), Optional.empty(), groupingId,
withInProjection, type, child()));
+ Optional.empty(), Optional.empty(), groupingId,
groupingIdValues, withInProjection, type, child()));
}
@Override
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalRepeat.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalRepeat.java
index 29d6da0abc9..85fc71e99f6 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalRepeat.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalRepeat.java
@@ -51,6 +51,7 @@ public class PhysicalRepeat<CHILD_TYPE extends Plan> extends
PhysicalUnary<CHILD
private final List<List<Expression>> groupingSets;
private final List<NamedExpression> outputExpressions;
private final SlotReference groupingId;
+ private final Optional<List<Long>> groupingIdValues;
/**
* Desc: Constructor for PhysicalRepeat.
@@ -61,6 +62,19 @@ public class PhysicalRepeat<CHILD_TYPE extends Plan> extends
PhysicalUnary<CHILD
SlotReference groupingId,
LogicalProperties logicalProperties,
CHILD_TYPE child) {
+ this(groupingSets, outputExpressions, groupingId, Optional.empty(),
logicalProperties, child);
+ }
+
+ /**
+ * Desc: Constructor for PhysicalRepeat with precomputed internal grouping
id values.
+ */
+ public PhysicalRepeat(
+ List<List<Expression>> groupingSets,
+ List<NamedExpression> outputExpressions,
+ SlotReference groupingId,
+ Optional<List<Long>> groupingIdValues,
+ LogicalProperties logicalProperties,
+ CHILD_TYPE child) {
super(PlanType.PHYSICAL_REPEAT, logicalProperties, child);
this.groupingSets = Objects.requireNonNull(groupingSets, "groupingSets
can not be null")
.stream()
@@ -69,13 +83,16 @@ public class PhysicalRepeat<CHILD_TYPE extends Plan>
extends PhysicalUnary<CHILD
this.outputExpressions = ImmutableList.copyOf(
Objects.requireNonNull(outputExpressions, "outputExpressions
can not be null"));
this.groupingId = Objects.requireNonNull(groupingId, "groupingId can
not be null");
+ this.groupingIdValues = groupingIdValues.map(ImmutableList::copyOf);
+ Preconditions.checkArgument(!this.groupingIdValues.isPresent()
+ || this.groupingIdValues.get().size() ==
this.groupingSets.size());
}
/**
* Desc: Constructor for PhysicalRepeat.
*/
private PhysicalRepeat(List<List<Expression>> groupingSets,
List<NamedExpression> outputExpressions,
- SlotReference groupingId,
+ SlotReference groupingId, Optional<List<Long>> groupingIdValues,
Optional<GroupExpression> groupExpression, LogicalProperties
logicalProperties,
PhysicalProperties physicalProperties, Statistics statistics,
CHILD_TYPE child) {
super(PlanType.PHYSICAL_REPEAT, groupExpression, logicalProperties,
@@ -87,6 +104,10 @@ public class PhysicalRepeat<CHILD_TYPE extends Plan>
extends PhysicalUnary<CHILD
this.outputExpressions = ImmutableList.copyOf(
Objects.requireNonNull(outputExpressions, "outputExpressions
can not be null"));
this.groupingId = Objects.requireNonNull(groupingId, "groupingId can
not be null");
+ this.groupingIdValues = groupingIdValues.map(ImmutableList::copyOf);
+ Preconditions.checkArgument(!this.groupingIdValues.isPresent()
+ || this.groupingIdValues.get().size() ==
this.groupingSets.size(),
+ "groupingIdValues size is different from groupingSets size");
}
@Override
@@ -103,6 +124,11 @@ public class PhysicalRepeat<CHILD_TYPE extends Plan>
extends PhysicalUnary<CHILD
return groupingId;
}
+ @Override
+ public Optional<List<Long>> getGroupingIdValues() {
+ return groupingIdValues;
+ }
+
@Override
public List<NamedExpression> getOutputs() {
return outputExpressions;
@@ -113,6 +139,7 @@ public class PhysicalRepeat<CHILD_TYPE extends Plan>
extends PhysicalUnary<CHILD
return Utils.toSqlString("PhysicalRepeat[" + id.asInt() + "]" +
getGroupIdWithPrefix(),
"groupingSets", groupingSets,
"outputExpressions", outputExpressions,
+ "groupingIdValues", groupingIdValues,
"stats", statistics
);
}
@@ -149,26 +176,27 @@ public class PhysicalRepeat<CHILD_TYPE extends Plan>
extends PhysicalUnary<CHILD
}
PhysicalRepeat that = (PhysicalRepeat) o;
return Objects.equals(groupingSets, that.groupingSets)
- && Objects.equals(outputExpressions, that.outputExpressions);
+ && Objects.equals(outputExpressions, that.outputExpressions)
+ && Objects.equals(groupingIdValues, that.groupingIdValues);
}
@Override
public int hashCode() {
- return Objects.hash(groupingSets, outputExpressions);
+ return Objects.hash(groupingSets, outputExpressions, groupingIdValues);
}
@Override
public PhysicalRepeat<Plan> withChildren(List<Plan> children) {
Preconditions.checkArgument(children.size() == 1);
return AbstractPlan.copyWithSameId(this, () -> new
PhysicalRepeat<>(groupingSets, outputExpressions,
- groupingId, groupExpression,
+ groupingId, groupingIdValues, groupExpression,
getLogicalProperties(), physicalProperties, statistics,
children.get(0)));
}
@Override
public PhysicalRepeat<CHILD_TYPE>
withGroupExpression(Optional<GroupExpression> groupExpression) {
return AbstractPlan.copyWithSameId(this, () -> new
PhysicalRepeat<>(groupingSets, outputExpressions,
- groupingId, groupExpression,
+ groupingId, groupingIdValues, groupExpression,
getLogicalProperties(), physicalProperties, statistics,
child()));
}
@@ -177,7 +205,7 @@ public class PhysicalRepeat<CHILD_TYPE extends Plan>
extends PhysicalUnary<CHILD
Optional<LogicalProperties> logicalProperties, List<Plan>
children) {
Preconditions.checkArgument(children.size() == 1);
return AbstractPlan.copyWithSameId(this, () -> new
PhysicalRepeat<>(groupingSets, outputExpressions,
- groupingId, groupExpression,
+ groupingId, groupingIdValues, groupExpression,
logicalProperties.get(), physicalProperties, statistics,
children.get(0)));
}
@@ -185,14 +213,14 @@ public class PhysicalRepeat<CHILD_TYPE extends Plan>
extends PhysicalUnary<CHILD
public PhysicalRepeat<CHILD_TYPE>
withPhysicalPropertiesAndStats(PhysicalProperties physicalProperties,
Statistics statistics) {
return AbstractPlan.copyWithSameId(this, () -> new
PhysicalRepeat<>(groupingSets, outputExpressions,
- groupingId, groupExpression,
+ groupingId, groupingIdValues, groupExpression,
getLogicalProperties(), physicalProperties, statistics,
child()));
}
@Override
public PhysicalRepeat<CHILD_TYPE> withAggOutput(List<NamedExpression>
newOutput) {
return AbstractPlan.copyWithSameId(this, () -> new
PhysicalRepeat<>(groupingSets, newOutput, groupingId,
- Optional.empty(),
+ groupingIdValues, Optional.empty(),
getLogicalProperties(), physicalProperties, statistics,
child()));
}
@@ -200,13 +228,13 @@ public class PhysicalRepeat<CHILD_TYPE extends Plan>
extends PhysicalUnary<CHILD
public PhysicalRepeat<CHILD_TYPE>
withGroupSetsAndOutput(List<List<Expression>> groupingSets,
List<NamedExpression> outputExpressionList) {
return AbstractPlan.copyWithSameId(this, () -> new
PhysicalRepeat<>(groupingSets, outputExpressionList,
- groupingId, Optional.empty(),
+ groupingId, groupingIdValues.filter(values -> values.size() ==
groupingSets.size()), Optional.empty(),
getLogicalProperties(), physicalProperties, statistics,
child()));
}
@Override
public PhysicalRepeat<CHILD_TYPE> resetLogicalProperties() {
- return new PhysicalRepeat<>(groupingSets, outputExpressions,
groupingId, groupExpression,
+ return new PhysicalRepeat<>(groupingSets, outputExpressions,
groupingId, groupingIdValues, groupExpression,
null, physicalProperties, statistics, child());
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/DecomposeRepeatWithPreAggregationTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/DecomposeRepeatWithPreAggregationTest.java
index 78cfdefe2c3..66434072777 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/DecomposeRepeatWithPreAggregationTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/DecomposeRepeatWithPreAggregationTest.java
@@ -27,6 +27,9 @@ import
org.apache.doris.nereids.trees.expressions.SlotReference;
import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
import org.apache.doris.nereids.trees.expressions.functions.agg.Max;
import org.apache.doris.nereids.trees.expressions.functions.agg.Sum;
+import org.apache.doris.nereids.trees.expressions.functions.scalar.Grouping;
+import org.apache.doris.nereids.trees.expressions.functions.scalar.GroupingId;
+import org.apache.doris.nereids.trees.expressions.literal.BigIntLiteral;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.algebra.Repeat.RepeatType;
import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
@@ -371,19 +374,20 @@ public class DecomposeRepeatWithPreAggregationTest
extends TestWithFeService imp
ImmutableList.of(a),
ImmutableList.of()
);
+ SlotReference groupingId = new SlotReference("grouping_id",
IntegerType.INSTANCE);
LogicalRepeat<Plan> repeat = new LogicalRepeat<>(
groupingSets,
(List) ImmutableList.of(a, b),
- new SlotReference("grouping_id", IntegerType.INSTANCE),
+ groupingId,
RepeatType.GROUPING_SETS,
emptyRelation);
LogicalAggregate<Plan> aggregate = new LogicalAggregate<>(
- ImmutableList.of(a, b),
- ImmutableList.of(a, b, sumAlias),
+ ImmutableList.of(a, b, groupingId),
+ ImmutableList.of(a, b, sumAlias, groupingId),
repeat);
LogicalProject<Plan> project = new LogicalProject<>(
- ImmutableList.of(a, b, sumAlias.toSlot()),
+ ImmutableList.of(a, b, sumAlias.toSlot(), groupingId),
aggregate);
LogicalCTEConsumer consumer = new LogicalCTEConsumer(
org.apache.doris.nereids.trees.expressions.StatementScopeIdGenerator.newRelationId(),
@@ -392,6 +396,7 @@ public class DecomposeRepeatWithPreAggregationTest extends
TestWithFeService imp
LogicalUnion result = (LogicalUnion) method.invoke(rule, project,
consumer, aggregate);
Assertions.assertNotNull(result);
Assertions.assertEquals(2, result.children().size());
+ Assertions.assertTrue(result.getOutputSet().contains(groupingId));
Assertions.assertTrue(aggregate.getOutputSet().containsAll(result.getOutputSet()));
}
@@ -443,6 +448,7 @@ public class DecomposeRepeatWithPreAggregationTest extends
TestWithFeService imp
LogicalRepeat.class,
org.apache.doris.nereids.trees.plans.logical.LogicalPlan.class,
List.class,
+ List.class,
Map.class,
List.class);
method.setAccessible(true);
@@ -484,11 +490,55 @@ public class DecomposeRepeatWithPreAggregationTest
extends TestWithFeService imp
new CTEId(1), "", new LogicalCTEProducer<>(new CTEId(1),
emptyRelation));
List<NamedExpression> groupingFunctionSlots = new ArrayList<>();
LogicalRepeat<Plan> result = (LogicalRepeat<Plan>) method.invoke(rule,
- originalRepeat, consumer, newGroupingSets,
producerToConsumerSlotMap, groupingFunctionSlots);
+ originalRepeat, consumer, newGroupingSets,
ImmutableList.of(1L, 3L),
+ producerToConsumerSlotMap, groupingFunctionSlots);
Assertions.assertNotNull(result);
Assertions.assertEquals(2, result.getGroupingSets().size());
Assertions.assertTrue(groupingFunctionSlots.isEmpty());
+ Assertions.assertEquals(ImmutableList.of(1L, 3L),
result.getGroupingIdValues().get());
+ Assertions.assertFalse(result.getGroupingId().get().nullable());
+ }
+
+ @Test
+ public void testDirectChildUsesGroupingScalarValuesFromResidualRepeat()
throws Exception {
+ Method method = rule.getClass().getDeclaredMethod("getDirectChild",
+ LogicalCTEConsumer.class, List.class, int.class);
+ method.setAccessible(true);
+
+ SlotReference a = new SlotReference("a", IntegerType.INSTANCE);
+ SlotReference b = new SlotReference("b", IntegerType.INSTANCE);
+ SlotReference c = new SlotReference("c", IntegerType.INSTANCE);
+ SlotReference d = new SlotReference("d", IntegerType.INSTANCE);
+ SlotReference e = new SlotReference("e", IntegerType.INSTANCE);
+ Alias groupingE = new Alias(new Grouping(e), "grouping_e");
+ Alias groupingId = new Alias(new GroupingId(a, b, c, d, e),
"grouping_id_abcde");
+ List<List<Expression>> groupingSets = ImmutableList.of(
+ ImmutableList.of(a, b, c, d),
+ ImmutableList.of(a, b, c),
+ ImmutableList.of(a, b),
+ ImmutableList.of(a),
+ ImmutableList.of()
+ );
+ LogicalEmptyRelation emptyRelation = new LogicalEmptyRelation(
+
org.apache.doris.nereids.trees.expressions.StatementScopeIdGenerator.newRelationId(),
+ ImmutableList.of());
+ LogicalRepeat<Plan> repeat = new LogicalRepeat<>(groupingSets,
+ ImmutableList.of(a, b, c, d, e, groupingE, groupingId),
+ new SlotReference("internal_grouping_id",
IntegerType.INSTANCE, false),
+ ImmutableList.of(1L, 3L, 7L, 15L, 31L), RepeatType.ROLLUP,
emptyRelation);
+
+ LogicalCTEConsumer consumer = new LogicalCTEConsumer(
+
org.apache.doris.nereids.trees.expressions.StatementScopeIdGenerator.newRelationId(),
+ new CTEId(1), "", new LogicalCTEProducer<>(new CTEId(1),
emptyRelation));
+ LogicalProject<Plan> directChild = (LogicalProject<Plan>)
method.invoke(rule,
+ consumer, repeat.computeGroupingFunctionsValues(), 0);
+
+ List<NamedExpression> projects = directChild.getProjects();
+ Assertions.assertEquals(3, projects.size());
+ Assertions.assertEquals(1L, ((BigIntLiteral)
projects.get(0).child(0)).getValue());
+ Assertions.assertEquals(1L, ((BigIntLiteral)
projects.get(1).child(0)).getValue());
+ Assertions.assertEquals(1L, ((BigIntLiteral)
projects.get(2).child(0)).getValue());
}
@Test
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/algebra/RepeatTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/algebra/RepeatTest.java
index 864fcc3e21d..77a0a64d93a 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/algebra/RepeatTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/algebra/RepeatTest.java
@@ -20,12 +20,14 @@ package org.apache.doris.nereids.trees.plans.algebra;
import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.Expression;
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.functions.scalar.GroupingId;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.algebra.Repeat.RepeatType;
import org.apache.doris.nereids.trees.plans.logical.LogicalOlapScan;
import org.apache.doris.nereids.trees.plans.logical.LogicalRepeat;
+import org.apache.doris.nereids.types.BigIntType;
import org.apache.doris.nereids.util.PlanConstructor;
import com.google.common.collect.ImmutableList;
@@ -203,4 +205,22 @@ public class RepeatTest {
// (id) -> index {0} -> slot id {3}
Assertions.assertEquals(Sets.newLinkedHashSet(ImmutableList.of(3)),
result.get(1));
}
+
+ @Test
+ public void testComputeGroupingFunctionsValuesUsePrecomputedGroupingIds() {
+ List<List<Expression>> groupingSets = ImmutableList.of(
+ ImmutableList.of(id),
+ ImmutableList.of()
+ );
+ Repeat<Plan> repeat = new LogicalRepeat<>(
+ groupingSets,
+ ImmutableList.of(id),
+ new SlotReference("GROUPING_ID", BigIntType.INSTANCE),
+ ImmutableList.of(1L, 3L),
+ RepeatType.GROUPING_SETS,
+ scan
+ );
+
+ Assertions.assertEquals(ImmutableList.of(1L, 3L),
repeat.computeGroupingFunctionsValues().get(0));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
