arnavahire19 opened a new pull request, #24743:
URL: https://github.com/apache/datafusion/pull/24743
Closes #24735.
### Problem
In `GroupedHashAggregateStream::process_batch`, `agg_start_time` is recorded
once before iterating over the accumulators. However,
`self.group_by_metrics.aggregation_time.add_elapsed(agg_start_time)` was placed
inside the accumulator loop.
For queries with $N$ aggregate functions, every iteration re-adds the
accumulated duration from the start of the loop (`sum_{k=1..N} (t_1 + ... +
t_k)`), inflating the reported `aggregation_time` metric by up to $(N+1)/2$x.
### Solution
Move `add_elapsed(agg_start_time)` outside and after the accumulator loop so
that the total time spent updating/merging accumulators for the batch is
recorded exactly once per batch. This matches the behavior in
`aggregate_hash_table/common.rs` and `aggregate_hash_table/common_ordered.rs`.
### Testing
Ran `cargo test --package datafusion-physical-plan --lib aggregates` (all
193 tests passing).
--
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]
