github-actions[bot] commented on code in PR #65880:
URL: https://github.com/apache/doris/pull/65880#discussion_r3628988253
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeRepeat.java:
##########
@@ -159,7 +160,8 @@ private static LogicalAggregate<Plan>
normalizeRepeat(LogicalRepeat<Plan> repeat
// rewrite grouping scalar function to virtual slots
// rewrite the arguments of agg function to slots
List<NamedExpression> normalizedAggOutput = Lists.newArrayList();
- List<NamedExpression> groupingFunctions = Lists.newArrayList();
+ // use a map to deduplicate grouping scalar functions in projection
+ Map<GroupingScalarFunction, NamedExpression> groupingFunctions =
Maps.newHashMap();
Review Comment:
This is the behavior this PR is introducing, but no test currently proves
the deduplication invariant. `NormalizeRepeatTest` has no identical
`Grouping`/`GroupingId` pair, and the existing regression query that repeats
one `GROUPING_ID` only checks rows/operator shape, which is unchanged if two
virtual columns are materialized. Please add a focused case such as
`GROUPING(a) AS g1, GROUPING(a) AS g2` (plus a nested repeated occurrence) and
assert that `LogicalRepeat`/translation emits one grouping-function alias/value
vector while both aggregate outputs keep their distinct original ExprIds and
reuse that slot.
--
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]
