You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "kosiew (via GitHub)" <gi...@apache.org> on 2026/08/27 01:56:05 UTC
[PR] Add per-aggregate metrics to non-grouped AggregateStream [datafusion]
kosiew opened a new pull request, #24706:
URL: https://github.com/apache/datafusion/pull/24706
## Which issue does this PR close?
* Part of #23570
## Rationale for this change
Non-grouped aggregation can spend significant time evaluating aggregate arguments and filters, updating or merging accumulators, and producing partial or final accumulator output. `AggregateStream` did not expose per-aggregate timing for this work, making it difficult to identify which aggregate expression or phase contributes to execution time.
This change adds aggregate-specific phase timing to non-grouped aggregation using the same metric naming and aggregate labels as the grouped aggregation path, so expressions such as `SUM(a)` and `SUM(b)` can be distinguished.
## What changes are included in this PR?
* Adds per-aggregate timing for argument and filter evaluation in `AggregateStream`.
* Adds timing around accumulator `update_batch` and `merge_batch` calls according to the aggregate input mode.
* Adds timing around accumulator `state` emission for partial output modes and `evaluate` for final output modes.
* Selects the accumulator metric phases exposed for each `AggregateMode`.
* Reuses `AggregateArgumentMetrics`, `AggregateAccumulatorMetrics`, `AccumulatorPhase`, and aggregate metric labels from the grouped aggregation metrics infrastructure.
* Keeps the existing baseline elapsed-compute timing around aggregate processing and finalization.
## Are these changes tested?
Yes. This PR adds:
* `accumulator_phases_match_aggregate_mode`, which verifies the expected accumulator phases for all six aggregate modes.
* `aggregate_stream_reports_per_aggregate_metrics`, which verifies that a single-stage non-grouped aggregate with `SUM(a)` and `SUM(b)` reports distinct `arguments`, `update`, and `evaluate` metrics and does not report unused `merge` or `state` phases.
* `aggregate_stream_reports_partial_and_final_phases`, which verifies that partial aggregation reports `arguments`, `update`, and `state`, while final aggregation reports `arguments`, `merge`, and `evaluate`.
The tests also verify that same-function aggregates over different columns are distinguishable by their aggregate labels.
## Are there any user-facing changes?
`AggregateStream` metrics now expose per-aggregate-expression timing for the phases executed by non-grouped aggregation. This provides additional observability for consumers of execution metrics, including `EXPLAIN ANALYZE`.
There are no public API changes shown in this patch.
## LLM-generated code disclosure
This PR includes LLM-generated code and comments. All LLM-generated content has been manually reviewed.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add per-aggregate metrics to non-grouped AggregateStream [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #24706:
URL: https://github.com/apache/datafusion/pull/24706#issuecomment-5433538176
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/24706?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `92.40506%` with `18 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 81.45%. Comparing base ([`872df4d`](https://app.codecov.io/gh/apache/datafusion/commit/872df4d8f0a818b8b3367032f41eb081f98b5d82?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`915d210`](https://app.codecov.io/gh/apache/datafusion/commit/915d210cc984727728e706f3c703fbfad39caee9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 5 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/24706?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...n/physical-plan/src/aggregates/aggregate\_stream.rs](https://app.codecov.io/gh/apache/datafusion/pull/24706?src=pr&el=tree&filepath=datafusion%2Fphysical-plan%2Fsrc%2Faggregates%2Faggregate_stream.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1wbGFuL3NyYy9hZ2dyZWdhdGVzL2FnZ3JlZ2F0ZV9zdHJlYW0ucnM=) | 92.40% | [5 Missing and 13 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/24706?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #24706 +/- ##
========================================
Coverage 81.45% 81.45%
========================================
Files 1119 1120 +1
Lines 400411 401080 +669
Branches 400411 401080 +669
========================================
+ Hits 326144 326702 +558
- Misses 55195 55253 +58
- Partials 19072 19125 +53
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/24706?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add per-aggregate metrics to non-grouped AggregateStream [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew merged PR #24706:
URL: https://github.com/apache/datafusion/pull/24706
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add per-aggregate metrics to non-grouped AggregateStream [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on PR #24706:
URL: https://github.com/apache/datafusion/pull/24706#issuecomment-5449651531
Thanks @rluvaton for your review.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org