ranflarion opened a new pull request, #24736: URL: https://github.com/apache/datafusion/pull/24736
## Which issue does this PR close? - Closes #24735. ## Rationale for this change `aggregation_time` on `GroupedHashAggregateStream` is added inside the per-accumulator loop while `agg_start_time` is taken once before it, so with N aggregate functions the k-th accumulator re-adds the elapsed time of all k accumulators so far, inflating the metric by up to (N+1)/2. On a 28-function GROUP BY a single task reported 4m22s of `aggregation_time` inside a 2m0s stage. Single-aggregate plans are unaffected. The newer `aggregate_hash_table` implementations already scope the timer around the whole loop. ## What changes are included in this PR? Move the `add_elapsed` call after the accumulator loop, once per interned batch. `time_calculating_group_ids`, which shares `agg_start_time` as its end point, is unchanged, and the per-accumulator `aggregate_accumulator_metrics` timings are unaffected. ## Are these changes tested? Covered by the existing aggregate tests (`cargo test -p datafusion-physical-plan aggregates`, 193 passed). The metric's value is wall-clock time, which the existing tests do not assert exactly; the inflation itself was measured on a 28-aggregate workload where the fixed metric now stays within the operator's elapsed time. ## Are there any user-facing changes? `aggregation_time` (shown in `EXPLAIN ANALYZE` as elapsed_compute subset time) now reports the actual time spent in accumulators on plans with more than one aggregate function; previously it over-reported on such plans. -- 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]
