alamb commented on code in PR #17805:
URL: https://github.com/apache/datafusion/pull/17805#discussion_r2388676582
##########
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:
this is a good clarification
If DataFusion ever supports more ordered set aggregation functions, we may
want to revisit this
In addition to saying what this setting doesn't do, maybe we could also say
what setting it to `true` does do? Specifically, it seems like it only affects
the output display somehow 🤔
--
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]
