kosiew opened a new pull request, #25748: URL: https://github.com/apache/datafusion/pull/25748
## Which issue does this PR close? - Part of #25394 ## Rationale for this change Replacing the aggregate expressions in an `AggregateExec` previously updated only the expression list. Properties derived from those expressions, including required input ordering and dynamic-filter metadata, could remain stale. This could leave the physical plan inconsistent. For example, replacing `array_agg(b)` with `array_agg(b ORDER BY b)` would not record the new ordering requirement, so the optimizer might not insert the required sort. Incompatible replacement schemas could also be accepted until execution. ## What changes are included in this PR? - Adds the fallible `AggregateExec::try_with_new_aggr_exprs` API. - Rebuilds `AggregateExec` through its constructor so ordering requirements, dynamic filters, metrics, and other expression-derived state are recomputed. - Validates that replacement expressions have the same output field count, data types, nullability, field metadata, and schema metadata as the retained schema. - Preserves existing output field names when only the replacement aliases differ. - Returns a planning error for incompatible replacement expressions. - Preserves aggregate filters and validates that the filter and aggregate-expression counts match. - Retains the distinct aggregation soft limit for empty-expression replacements. - Deprecates `with_new_aggr_exprs` and delegates it to the fallible replacement path. - Updates `OptimizeAggregateOrder` to use the fallible API and propagate replacement errors. ## Are these changes tested? Yes. This PR adds the following regression tests: - `replacement_order_requirement_is_enforced` - `empty_aggregate_replacement_retains_distinct_soft_limit` - `replacement_recomputes_ordering_requirements` - `deprecated_replacement_api_delegates_to_fallible_replacement` - `replacement_rebuilds_dynamic_filter` - `replacement_validates_schema_except_output_names` The tests cover recomputing and enforcing aggregate `ORDER BY` requirements, rebuilding dynamic-filter kinds, indexes, input columns, identities, and mutable bounds, removing dynamic filtering for unsupported aggregates, validating schema compatibility, preserving output names, and retaining the distinct soft limit. The existing optimizer test helper is also strengthened to assert that the optimized aggregate retains its input-ordering requirement. ## Are there any user-facing changes? There are no changes to SQL syntax or configuration. This fixes internal physical-plan replacement behavior so expression-derived requirements remain consistent and incompatible replacements fail during planning. For Rust API consumers, `try_with_new_aggr_exprs` is added as the fallible replacement API, while `with_new_aggr_exprs` is deprecated but remains available. ## 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]
