alamb commented on code in PR #16625: URL: https://github.com/apache/datafusion/pull/16625#discussion_r2217293999
########## datafusion/ffi/src/udaf/mod.rs: ########## @@ -561,6 +561,7 @@ impl AggregateUDFImpl for ForeignAggregateUDF { pub enum FFI_AggregateOrderSensitivity { Insensitive, HardRequirement, + SoftRequirement, Review Comment: Technically speaking this is an FFI API change -- I am not sure what implication that has (note this would not be released until DataFusion 50 anyways). cc @timsaucer -- I wonder if we should gather up the FFI breaking changes into their own PR / more carefully schedule such breakages ########## datafusion/functions-aggregate-common/src/order.rs: ########## @@ -25,6 +25,10 @@ pub enum AggregateOrderSensitivity { /// The aggregator can not produce a correct result unless its ordering /// requirement is satisfied. HardRequirement, + /// Indicates that the aggregate expression strongly prefers the input to be ordered. + /// The aggregator can produce its result correctly regardless of the input ordering, + /// This is a similar to, but stronger than, `Beneficial`. Review Comment: I think it would help to explain why the aggregate expression might prefer the input to be ordered. Maybe we can incorporate the comments from @ozankabak here: https://github.com/apache/datafusion/pull/16625#issuecomment-3031665846 Something like ```suggestion /// Indicates that the aggregate expression strongly prefers the input to be ordered. /// Similarly to [`Self::HardRequirement`], when possible DataFusion will insert /// a `SortExec`, to reorder the input to match the SoftRequirement. However, /// when such a sort cannot be inserted, no error will be thrown, unlike with /// with [`Self::HardRequirement`] /// /// The aggregator can produce its result correctly regardless of the input ordering, /// This is a similar to, but stronger than, `Beneficial`. ``` -- 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