fix: preserve output field name when reversing aggregate window expressions

[timsaucer]

Which issue does this PR close?

• Closes Window reversal renames output field for aggregate-UDAF window functions, breaking parent projection #24884.

Rationale for this change

Using an aggregate UDAF as a window function (reachable through the DataFrame API) fails to plan whenever the physical optimizer decides to reverse the window to avoid an extra sort:

EnsureRequirements
caused by
Internal error: Assertion failed: col.name() == matching_name: Input field name
first_value(?table?.v) ORDER BY [?table?.t ASC NULLS FIRST] ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW
does not match with the projection expression
last_value(?table?.v) ORDER BY [?table?.t DESC NULLS LAST] ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW.

get_best_fitting_window swaps in WindowExpr::get_reverse_expr(), and for aggregate-backed window expressions that reaches AggregateFunctionExpr::reverse_expr, which rewrites the aggregate's output name (last_value(v) ORDER BY [t DESC] → first_value(v) ORDER BY [t ASC]). A window exec derives its schema from WindowExpr::field(), so the node's output column gets renamed while the parent ProjectionExec still holds a Column with the old name.

Renaming on reversal only makes sense for AggregateExec, which pins its schema at construction (try_new builds the schema before reversing exprs) — there the renamed name is a useful signal in EXPLAIN of which implementation actually runs, and no parent references it. The window path has the opposite requirement, which is why WindowUDFExpr::reverse_expr already carries name over unchanged. This makes the aggregate-backed window path behave the same way.

The equivalent SQL query does not fail, because in SQL last_value(v) OVER (...) resolves to the last_value window UDF, which reverses without renaming.

Scope

Reviewed the rest of this bug class while here:

• first_value ↔ last_value is the only ReversedUDAF::Reversed pair with a different name; array_agg, string_agg and nth_value reverse to themselves, so the rewrite is a no-op for them.
• Two rules reach get_best_fitting_window — enforce_sorting and enforce_distribution — so the fix is applied at the get_reverse_expr level to cover both.
• OptimizeAggregateOrder also calls reverse_expr, but AggregateExec::with_new_aggr_exprs keeps the original schema, so it cannot rename an output field.
• OptimizationInvariantChecker does compare field names, but only for the root plan schema, so a rename on an intermediate node under a name-preserving projection is invisible to it. Hence the local assertion below.

What changes are included in this PR?

• AggregateFunctionExpr::reverse_expr is refactored into reverse_expr_inner(preserve_name), with a new public reverse_expr_preserving_name(). Existing reverse_expr behavior is unchanged.
• PlainAggregateWindowExpr::get_reverse_expr and SlidingAggregateWindowExpr::get_reverse_expr use the name-preserving variant. Their bodies were byte-identical, so they are factored into a shared reverse_aggregate_window_expr helper.
• get_best_fitting_window now asserts that reversal did not change any output field name, so a future renaming WindowExpr implementation fails there — naming the culprit — instead of at a distant ProjectionMapping assertion.

Note that the prefix-based replace_fn_name_clause also mangled user-supplied aliases beginning with last_value/first_value (e.g. an alias last_value_desc became first_value_desc); that is fixed on the window path too.

What is the testing strategy for this PR?

Two new regression tests, both verified to fail without the fix and pass with it:

• datafusion/core/tests/dataframe/mod.rs — window_reversal_preserves_output_field_names reproduces the issue through the DataFrame API and reproduces the reported error exactly. It asserts on create_physical_plan() rather than collect(), because execution then hits the separate missing-retract_batch gap tracked by Support first_value/last_value aggregates as sliding window aggregates (blocks FILTER + windowed first/last value) #24885; a comment marks where to upgrade the assertion once that lands.
• datafusion/core/tests/physical_optimizer/window_optimize.rs — test_window_reversal_preserves_output_field_names builds two opposite-ORDER BY aggregate-UDAF windows under a projection, runs EnsureRequirements, and asserts the optimized plan's schema field names are unchanged. Without the fix it fails with Input field name first_value_desc does not match with the projection expression last_value_desc.

The existing test_reverse_expr_preserves_non_aliased_display_path and the two neighboring reverse_expr display tests still pass, confirming the plain-aggregate renaming is untouched. All 510 sqllogictest files pass with no expectation changes — SQL cannot reach the renaming branch, and existing reversal expectations already show the original name with a reversed frame, which is exactly the shape this fix produces.

Full extended suite (--features avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption): 11055 passed, 0 failed. cargo clippy --all-targets --all-features -- -D warnings clean.

Are there any user-facing changes?

Queries that previously failed to plan now plan successfully; no expected output or plan text changes for anything that worked before. AggregateFunctionExpr::reverse_expr_preserving_name is a new public method — additive, no breaking API changes.

🤖 Generated with Claude Code

[codecov-commenter]

Codecov Report

❌ Patch coverage is 95.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.64%. Comparing base (20d1c56) to head (f271a9c).

Files with missing lines	Patch %	Lines
datafusion/physical-plan/src/windows/mod.rs	57.14%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #24887   +/-   ##
=======================================
  Coverage   81.64%   81.64%           
=======================================
  Files        1123     1123           
  Lines      410248   410253    +5     
  Branches   410248   410253    +5     
=======================================
+ Hits       334940   334952   +12     
+ Misses      55617    55609    -8     
- Partials    19691    19692    +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[timsaucer]

This is really the crux of this PR. There is a bit of refactoring to take common code from datafusion/physical-expr/src/window/sliding_aggregate.rs and also datafusion/physical-expr/src/window/aggregate.rs so that they treat it identically. Then we just need to pass around that boolean that says whether or not to preserver the name after rewriting.