github-actions[bot] commented on code in PR #65968:
URL: https://github.com/apache/doris/pull/65968#discussion_r3682688597
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/DecomposeRepeatWithPreAggregation.java:
##########
@@ -151,45 +151,49 @@ public Plan visitLogicalAggregate(LogicalAggregate<?
extends Plan> aggregate, Di
LogicalRepeat<Plan> repeat = (LogicalRepeat<Plan>) aggregate.child();
List<List<Expression>> newGroupingSets = new ArrayList<>();
+ List<Long> originalGroupingIdValues = repeat.getGroupingIdValues()
+ .orElseGet(() -> repeat.toShapes().computeGroupingIdValue());
+ 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,
groupingFunctionSlots,
+ originalGroupingIdValues.get(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 original grouping id and zero literals for the grouping
function slots.
*
* @param directConsumer the CTE consumer for the direct path
* @param groupingFunctionSlots the list of grouping function slots to
handle
+ * @param groupingIdValue original internal grouping id for 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<NamedExpression> groupingFunctionSlots,
+ long groupingIdValue) {
+ ImmutableList.Builder<NamedExpression> builder =
ImmutableList.builder();
+ builder.addAll(directConsumer.getOutput());
+ builder.add(new Alias(new BigIntLiteral(groupingIdValue)));
+ for (int i = 0; i < groupingFunctionSlots.size(); ++i) {
+ builder.add(new Alias(new BigIntLiteral(0)));
Review Comment:
[P1] Preserve scalar grouping values on repeated decomposition
This direct branch is not always the original all-columns grouping set.
After a supported CTE-inline plus CBO-MV rewrite, the residual Repeat can be
decomposed again. For example, with `ROLLUP(a,b,c,d,e)` and `GROUPING(e)`, the
first pass removes `(a,b,c,d,e)`, so the residual maximum `(a,b,c,d)` correctly
has `GROUPING(e) = 1`; the second pass reaches this loop and emits `0`. SQL
`GROUPING_ID(...)` is wrong for the same reason when any argument existed only
in the previously removed set. Please carry/derive each current per-set
grouping-scalar value and use the value at `maxGroupIndex` instead of assuming
zero.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
