findepi commented on code in PR #16625: URL: https://github.com/apache/datafusion/pull/16625#discussion_r2217667332
########## datafusion/ffi/src/udaf/mod.rs: ########## @@ -561,6 +561,7 @@ impl AggregateUDFImpl for ForeignAggregateUDF { pub enum FFI_AggregateOrderSensitivity { Insensitive, HardRequirement, + SoftRequirement, Review Comment: This new thing doesn't need to be supported in the FFI. However, i didn't know how to avoid adding this. When looking at `impl From<AggregateOrderSensitivity> for FFI_AggregateOrderSensitivity` i am under impression that this particular part of FFI API is tightly coupled with the datafusion core, so in this particular place it cannot deliver API stability without inhibiting datafusion core progress. The necessary solution might be replacing this `From` with `TryFrom`, same with (`impl From<FFI_AggregateOrderSensitivity> for AggregateOrderSensitivity)`. My understanding is that, by tightly coupling `AggregateOrderSensitivity` and `FFI_AggregateOrderSensitivity` code author chose to let these enums naturally evolve over time, considering this not a breaking change, or an acceptable breaking change. -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org