kosiew opened a new pull request, #24024:
URL: https://github.com/apache/datafusion/pull/24024
#### Which issue does this PR close?
* Part of #23570
---
## Rationale for this change
Grouped hash aggregation currently exposes a single
`aggregate_arguments_time` metric for all aggregate argument evaluation. This
makes it difficult to determine which aggregate expression is responsible for
argument evaluation cost when multiple aggregates are present (for example,
`SUM(a)` versus `SUM(b)`).
This change adds per-aggregate argument evaluation metrics while preserving
the existing operator-level metric for compatibility.
---
## What changes are included in this PR?
* Add an `AggregateArgumentMetrics` helper that registers one timer per
aggregate expression during operator construction.
* Label per-aggregate metrics using the aggregate expression display text
(or alias), allowing otherwise identical aggregate functions on different
inputs (for example, `SUM(a)` and `SUM(b)`) to be distinguished.
* Wrap aggregate argument evaluation with the corresponding per-aggregate
timer in:
* grouped hash aggregation stream
* migrated hash table aggregation paths
* ordered aggregation table paths
* Introduce `OrderedAggregateTableMetrics` to carry both group-by and
aggregate argument metrics through ordered aggregation replay/spill paths.
* Factor out a reusable `aggregate_metric_label` helper for consistent
metric labels.
* Preserve the existing `aggregate_arguments_time` metric so whole-phase
timing continues to be reported.
---
## Are these changes tested?
Yes.
This PR adds and updates tests including:
* `test_groupby_aggregate_argument_metrics_distinguish_inputs`, which
verifies that separate metrics are created for `SUM(a)` and `SUM(b)` with
distinct metric names and aggregate labels.
* Existing group-by metrics tests updated to use shared aggregate
construction helpers.
* Grouped hash aggregation stream tests updated to verify both:
* the existing `aggregate_arguments_time` metric remains present and
non-zero, and
* the new per-aggregate metric (`agg_expr_0_arguments_time`) is emitted
and records time.
---
## Are there any user-facing changes?
Yes.
`EXPLAIN ANALYZE` and execution plan metrics for the covered grouped hash
aggregation paths now include per-aggregate argument evaluation timers (for
example, `agg_expr_0_arguments_time`) with aggregate labels identifying the
corresponding aggregate expression, while retaining the existing
`aggregate_arguments_time` metric for compatibility.
---
## 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: [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]
