fix: duplicate ordering state field names in partial aggregates

[cdelmonte-zg]

Which issue does this PR close?

• Closes Partial AggregateMode will generate duplicate field names from state_fields #17715.

Rationale for this change

Partial aggregate state schemas can contain duplicate field names when multiple order-sensitive aggregate expressions use the same ordering expression.

For example, first_value(value ORDER BY timestamp) and last_value(value ORDER BY timestamp) both exposed the ordering state field as timestamp@0.

This required DFSchema::try_from to skip check_names() as a workaround. However, the state_fields() contract requires state field names to be unique within the query.

What changes are included in this PR?

• Namespace ordering state fields with the aggregate name and the ordering-field position.
• Apply the same naming scheme to the default AggregateUDFImpl::state_fields() implementation and to FirstValue / LastValue.
• Re-enable DFSchema::check_names() in TryFrom<SchemaRef> for DFSchema.
• Update the partial aggregate regression test to assert unique state field names.
• Add a unit test for the default state_fields() implementation, including duplicate ordering field names within a single aggregate.

For example, an ordering state field now has a name such as:

first_value(value)[ordering_0_timestamp@0]

rather than the unqualified:

timestamp@0

The ordering position also disambiguates repeated ordering fields within the same aggregate.

What is the testing strategy for this PR?

The change is covered by:

• test_partial_aggregate_state_fields_have_unique_names
• test_default_state_fields_namespaces_ordering_fields

The following test suites pass locally:

cargo test -p datafusion-expr
cargo test -p datafusion-functions-aggregate
cargo test -p datafusion-physical-plan
cargo test -p datafusion-common
cargo test -p datafusion --test core_integration
cargo test -p datafusion-ffi --features integration-tests

cargo fmt --all -- --check and git diff --check also pass.

Are there any user-facing changes?

No public API signatures were changed. Ordering state field names now use the aggregate namespace and ordering position to guarantee uniqueness, allowing DFSchema::check_names() to be re-enabled.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 77.04918% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.91%. Comparing base (0da2151) to head (13d4a75).
⚠️ Report is 9 commits behind head on main.

Files with missing lines	Patch %	Lines
datafusion/expr/src/udaf.rs	70.21%	13 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #25196      +/-   ##
==========================================
- Coverage   81.92%   81.91%   -0.01%     
==========================================
  Files        1132     1132              
  Lines      421192   421371     +179     
  Branches   421192   421371     +179     
==========================================
+ Hits       345041   345183     +142     
- Misses      55756    55778      +22     
- Partials    20395    20410      +15     

☔ 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.

[jayzhan211]

Thanks @cdelmonte-zg, all suggestions are non-blocking

Maybe we could also add the change to docs/source/library-user-guide/upgrading/56.0.0.md

[jayzhan211]

could we add helper to reduce duplication

/// Namespaces ordering fields so state field names stay unique per query.
pub fn ordering_state_fields(name: &str, ordering_fields: &[FieldRef]) -> Vec<FieldRef> {
    ordering_fields
        .iter()
        .enumerate()
        .map(|(idx, f)| {
            Arc::new(f.as_ref().clone().with_name(format_state_name(
                name,
                &format!("ordering_{idx}_{}", f.name()),
            )))
        })
        .collect()
}

[jayzhan211]

I think the first idx might be enough 🤔

first_value(value)[ordering_0_timestamp@0] becomes first_value(value)[ordering_0]