github-actions[bot] commented on code in PR #65846:
URL: https://github.com/apache/doris/pull/65846#discussion_r3620544547
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatus.java:
##########
@@ -326,7 +326,7 @@ private PreAggStatus
checkAggregateFunctions(Set<AggregateFunction> aggregateFun
}
PreAggStatus preAggStatus = PreAggStatus.on();
for (AggregateFunction aggFunc : aggregateFuncs) {
- if (aggFunc.children().isEmpty()) {
+ if (aggFunc.children().isEmpty() || aggFunc.children().size()
> 1) {
Review Comment:
[P2] Preserve preaggregation for all-key multi-column DISTINCT
This arity gate also turns preaggregation off for a safe plan:
LogicalAggregate(count(DISTINCT k5, k6))
LogicalOlapScan(preagg_t1 AGG_KEYS; k5 and k6 are key columns)
`SetPreAggStatus` sees the original two-child `Count`, so this branch now
returns OFF before classifying either slot. Partial AGG_KEYS rows can only
repeat the same key tuple, and the later DISTINCT phases remove those
duplicates, so ON is correct here (the previous
`KeyAndValueSlotsAggChecker.visitCount` path allowed it). OFF forces a full
storage merge and prevents an otherwise eligible AGG/UNIQUE-MOR scan from using
the parallel-scan path. Please validate each supported multi-child
`Count(DISTINCT ...)` argument relevant to the current scan using the existing
supported-expression rules, preserving ON for the direct all-key `k5, k6` case
and keeping value, unknown, or unsupported inputs OFF. Add an all-key positive
EXPLAIN case and make the mixed `k6, v7` case assert its aggregate-specific OFF
reason.
--
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]
