Fix: Show aliased aggregate expressions in physical EXPLAIN output#19693
Closed
GaneshPatil7517 wants to merge 1 commit intoapache:mainfrom
Closed
Fix: Show aliased aggregate expressions in physical EXPLAIN output#19693GaneshPatil7517 wants to merge 1 commit intoapache:mainfrom
GaneshPatil7517 wants to merge 1 commit intoapache:mainfrom
Conversation
github-actions
bot
added
documentation
crepererum
reviewed
Jan 9, 2026
Contributor
crepererum
left a comment
crepererum
left a comment
There was a problem hiding this comment.
Either I am misunderstanding the PR, or it is misnamed, or it accidentally includes a bunch of unrelated changes
| let (name, human_display, e) = match e { | ||
| Expr::Alias(Alias { name, .. }) => { | ||
| let unaliased = e.clone().unalias_nested().data; | ||
| (Some(name.clone()), e.human_display().to_string(), unaliased) |
Contributor
There was a problem hiding this comment.
this seems to be the core change that also matches the PR title
Contributor
There was a problem hiding this comment.
why are these changes included in this PR?
Contributor
There was a problem hiding this comment.
same: these changes don't seem to belong to this PR or I am missing the connection of the PR title to the reasoning.
| let a: Vec<String> = self | ||
| .aggr_expr | ||
| .iter() | ||
| .map(|agg| agg.name().to_string()) |
| 4 tag2 90 75 80 95 | ||
| 5 tag2 100 80 80 100 | ||
|
|
||
| ########### |
Contributor
There was a problem hiding this comment.
doesn't belong to this PR?
| select percentile_cont(null, 0.5); | ||
| ---- | ||
| NULL | ||
|
|
Contributor
There was a problem hiding this comment.
also doesn't belong here?
| [`create_udaf`]: https://docs.rs/datafusion/latest/datafusion/logical_expr/fn.create_udaf.html | ||
| [`advanced_udaf.rs`]: https://github.com/apache/datafusion/blob/main/datafusion-examples/examples/udf/advanced_udaf.rs | ||
|
|
||
| ### Nullability of Aggregate Functions |
Contributor
Author
There was a problem hiding this comment.
Apologies for the confusion! I accidentally created the branch from the wrong base.
…19685) When an aggregate expression has been aliased, the logical plan EXPLAIN shows both the alias and the original expression. However, the physical plan EXPLAIN only showed the alias, making plans hard to interpret. This fix updates the physical EXPLAIN output to show both the underlying aggregate expression and its alias in the format: AggregateExec: mode=Single, gby=[], aggr=[sum(column1@0) as my_alias] instead of: AggregateExec: mode=Single, gby=[], aggr=[my_alias] Changes: - Modified create_aggregate_expr_and_maybe_filter() in physical_planner.rs to use the unaliased expression for human_display, so it captures the actual aggregate expression instead of just the alias name. - Modified DisplayAs impl for AggregateExec to show both expression and alias when they differ. - Updated test expectations in explain.slt, aggregate.slt, and agg_func_substitute.slt to reflect the new output format.
GaneshPatil7517
force-pushed
the
fix-explain-aggregate-alias-visibility
branch
from
eeb1ede to
4ef18bd
Compare
github-actions
bot
removed
documentation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #19685
Rationale for this change
When an aggregate expression has been aliased, the logical plan
EXPLAINshows both the alias and the original expression. However, the physical planEXPLAINonly showed the alias name, making plans hard to interpret.Before:
AggregateExec: mode=Single, gby=[], aggr=[agg]
AggregateExec: mode=Single, gby=[], aggr=[sum(column1@0) FILTER (WHERE column2@1 <= Int64(0)) as agg]
What changes are included in this PR?
datafusion/core/src/physical_planner.rs: Modifiedcreate_aggregate_expr_and_maybe_filter()to use the unaliased expression forhuman_display, so it captures the actual aggregate expression instead of just the alias name.datafusion/physical-plan/src/aggregates/mod.rs: ModifiedDisplayAsimpl forAggregateExecto show both the expression (human_display) and alias (name()) when they differ, in the format{expression} as {alias}.Test file updates: Updated test expectations in
explain.slt,aggregate.slt, andagg_func_substitute.sltto reflect the new output format.Are these changes tested?
Yes. Added new test cases in
explain.sltthat specifically verify aliased aggregate expressions are visible in physical EXPLAIN output. Updated existing test expectations inaggregate.sltandagg_func_substitute.slt.Are there any user-facing changes?
Yes. The physical plan
EXPLAINoutput now shows the full aggregate expression along with its alias when they differ, improving plan readability and debuggability.