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

Reply via email to