bharadwaj-pendyala opened a new pull request, #25876: URL: https://github.com/apache/datafusion/pull/25876
## Which issue does this PR close? - Closes #25374. ## Rationale for this change `AggregateUDFImpl::distinct_handling` (#25288) doesn't survive the FFI boundary. `FFI_AggregateUDF` has no slot for it, so `ForeignAggregateUDF` falls back to the trait default and every UDAF loaded from another library reports `Sensitive`. A foreign `min` never gets its `DISTINCT` dropped, and a foreign `stddev`, which doesn't read `is_distinct` at all, tells the planner it deduplicates its own input. ## What changes are included in this PR? Same shape as `order_sensitivity`, all in `datafusion/ffi/src/udaf/mod.rs`: - `FFI_DistinctHandling`, a `#[repr(C)]` enum with `From` impls both ways, next to `FFI_AggregateOrderSensitivity`. - A `distinct_handling` fn pointer on `FFI_AggregateUDF`, added at the end of the struct after `supports_null_handling_clause`. - `ForeignAggregateUDF::distinct_handling` calls through it. `DistinctHandling` is `#[non_exhaustive]`, so the native-to-FFI conversion needs a wildcard arm. I map anything unknown to `Sensitive`, the trait default, which is what the issue suggested. That and appending the field at the end are the two assumptions worth a second look. The arm covers a variant added to `datafusion-expr` before the FFI enum catches up. Mismatched library versions stay unsupported, same as today. ## What is the testing strategy for this PR? - `udaf::tests::test_distinct_handling` wraps `min`, `sum` and `stddev` with the mock foreign marker and checks all three variants come back. On main `min` reports `Sensitive` and the test fails. - `tests/ffi_udaf.rs::test_distinct_handling` does the same through the separately built test library, for `stddev` (`Unsupported`) and `sum` (`Sensitive`). On main's `udaf/mod.rs` it fails with `left: Sensitive, right: Unsupported`. - `test_round_trip_all_distinct_handlings` round-trips each variant, like the existing order sensitivity test. `cargo test -p datafusion-ffi --features integration-tests` passes, as does `cargo clippy -p datafusion-ffi --all-targets --features integration-tests --no-deps -- -D warnings` and `cargo fmt --all -- --check`. ## Are there any user-facing changes? Foreign UDAFs now report the `distinct_handling` they declare, so an `Insensitive` one can have `DISTINCT` eliminated. Adding a field changes the `FFI_AggregateUDF` layout, so this probably wants the `api change` label. This was written with AI assistance. I've read the change end to end and reproduced both failures above myself. -- 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]
