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)]) with From impls in both directions, alongside
FFI_AggregateOrderSensitivity at datafusion/ffi/src/udaf/mod.rs:628.
- Add a
distinct_handling fn pointer to FFI_AggregateUDF and a distinct_handling_fn_wrapper,
mirroring order_sensitivity_fn_wrapper (datafusion/ffi/src/udaf/mod.rs:328).
- Implement
distinct_handling on ForeignAggregateUDF as
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 a ForeignAggregateUDF test asserting a foreign UDAF
that declares Insensitive reports Insensitive.
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.
Is your feature request related to a problem or challenge?
#25288 adds
AggregateUDFImpl::distinct_handling(datafusion/expr/src/udaf.rs:958), which lets anaggregate declare whether
DISTINCTcan change its result, andEliminateAggregateDistinctuses thatto drop the modifier from duplicate-insensitive functions.
ForeignAggregateUDF(datafusion/ffi/src/udaf/mod.rs:471) does not forward the new method, andFFI_AggregateUDF(datafusion/ffi/src/udaf/mod.rs:59) has no field to carry it, so every UDAF loadedover 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-likeor set-valued aggregate that declares
DistinctHandling::Insensitivestill gets the fullSingleDistinctToGroupBytreatment — a per-group hash set and an extra grouping stage that onlydeduplicates 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_sensitivityalready does:FFI_DistinctHandling(#[repr(C)]) withFromimpls in both directions, alongsideFFI_AggregateOrderSensitivityatdatafusion/ffi/src/udaf/mod.rs:628.distinct_handlingfn pointer toFFI_AggregateUDFand adistinct_handling_fn_wrapper,mirroring
order_sensitivity_fn_wrapper(datafusion/ffi/src/udaf/mod.rs:328).distinct_handlingonForeignAggregateUDFasunsafe { (self.udaf.distinct_handling)(&self.udaf).into() }.test_round_trip_all_order_sensitivities(
datafusion/ffi/src/udaf/mod.rs:864), plus aForeignAggregateUDFtest asserting a foreign UDAFthat declares
InsensitivereportsInsensitive.One wrinkle worth deciding:
DistinctHandlingis#[non_exhaustive], so theFrom<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
Sensitivekeeps it conservative — that is the default, and it onlyever 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 thesame function, which is surprising and hard to notice.