Jefffrey commented on code in PR #17805:
URL: https://github.com/apache/datafusion/pull/17805#discussion_r2390290537
##########
datafusion/expr/src/udaf.rs:
##########
@@ -742,18 +742,23 @@ pub trait AggregateUDFImpl: Debug + DynEq + DynHash +
Send + Sync {
/// If this function is ordered-set aggregate function, return true
/// otherwise, return false
///
- /// Ordered-set aggregate functions require an explicit `ORDER BY` clause
- /// because the calculation performed by these functions is dependent on
the
- /// specific sequence of the input rows, unlike other aggregate functions
- /// like `SUM`, `AVG`, or `COUNT`.
+ /// Ordered-set aggregate functions allow an `ORDER BY` clause because the
+ /// calculation performed by these functions is dependent on the specific
+ /// sequence of the input rows, unlike other aggregate functions like `SUM`
+ /// `AVG`, or `COUNT`. If explicit order is specified then a default order
+ /// of ascending is assumed.
///
- /// An example of an ordered-set aggregate function is `percentile_cont`
- /// which computes a specific percentile value from a sorted list of
values, and
- /// is only meaningful when the input data is ordered.
+ /// Note that setting this to `true` does not guarantee input sort order to
Review Comment:
> In addition to saying what this setting doesn't do, maybe we could also
say what setting it to `true` does do?
This is a good point 🤔
Let me look into the code a bit more to clarify my understanding and I'll
update the doc accordingly.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
