Repository navigation
Description
Is your feature request related to a problem or challenge?
#25288 adds AggregateUDFImpl::distinct_handling (datafusion/expr/src/udaf.rs:958), which lets an
aggregate declare whether DISTINCT can change its result, and EliminateAggregateDistinct uses that
to drop the modifier from duplicate-insensitive functions.
ForeignAggregateUDF (datafusion/ffi/src/udaf/mod.rs:471) does not forward the new method, and
FFI_AggregateUDF (datafusion/ffi/src/udaf/mod.rs:59) has no field to carry it, so every UDAF loaded
over FFI falls back to the default DistinctHandling::Sensitive.
The effect is that the new API silently does nothing across the FFI boundary. A third-party min-like
or set-valued aggregate that declares DistinctHandling::Insensitive still gets the full
SingleDistinctToGroupBy treatment — a per-group hash set and an extra grouping stage that only
deduplicates input the function was going to ignore — while the identical function registered
in-process does not. Results stay correct either way; the optimization is just unreachable.
Describe the solution you'd like
Carry the tag through the vtable the way order_sensitivity already does:
- Add
FFI_DistinctHandling(#[repr(C)]) withFromimpls in both directions, alongside
FFI_AggregateOrderSensitivityatdatafusion/ffi/src/udaf/mod.rs:628. - Add a
distinct_handlingfn pointer toFFI_AggregateUDFand adistinct_handling_fn_wrapper,
mirroringorder_sensitivity_fn_wrapper(datafusion/ffi/src/udaf/mod.rs:328). - Implement
distinct_handlingonForeignAggregateUDFas
unsafe { (self.udaf.distinct_handling)(&self.udaf).into() }. - Add a round-trip test over every variant, like
test_round_trip_all_order_sensitivities
(datafusion/ffi/src/udaf/mod.rs:864), plus aForeignAggregateUDFtest asserting a foreign UDAF
that declaresInsensitivereportsInsensitive.
One wrinkle worth deciding: DistinctHandling is #[non_exhaustive], so the From<FFI_DistinctHandling>
conversion needs a policy for a variant added by a newer library on the other side of the boundary.
Mapping anything unrecognized to Sensitive keeps it conservative — that is the default, and it only
ever costs an optimization, never correctness.
Describe alternatives you've considered
Leaving it as is. FFI aggregates keep working and return correct results; they just never benefit from
EliminateAggregateDistinct. The cost is that the two registration paths behave differently for the
same function, which is surprising and hard to notice.
Activity
[-]Title: ForeignAggregateUDF does not forward distinct_handling[/-][+]ForeignAggregateUDF does not forward distinct_handling[/+]mkleen commentedon Sep 16, 2026
take
[-]ForeignAggregateUDF does not forward distinct_handling[/-][+]ForeignAggregateUDF does not forward distinct handling[/+]mkleen commentedon Sep 29, 2026
untake
bharadwaj-pendyala commentedon Sep 29, 2026
I'd like to pick this up now that it's free. I've got a test in
datafusion/ffi/src/udaf/mod.rsthat wrapsminthrough the mock foreign marker, and on main it comes backSensitiveinstead ofInsensitive.Plan is what you laid out: an
FFI_DistinctHandlingenum next toFFI_AggregateOrderSensitivity, adistinct_handlingfn pointer onFFI_AggregateUDF, and the forwarding impl onForeignAggregateUDF. SinceDistinctHandlingis#[non_exhaustive], a variant the FFI side doesn't know yet maps toSensitive, so a mismatched library can only lose the optimization, never get a wrong answer. The new field goes at the end of the struct, aftersupports_null_handling_clause, same as the last addition.fix(ffi): forward distinct_handling through FFI_AggregateUDF (apache#…