Garamda commented on code in PR #13511: URL: https://github.com/apache/datafusion/pull/13511#discussion_r1976304218
########## datafusion/functions-aggregate/src/approx_percentile_cont.rs: ########## @@ -51,29 +52,43 @@ create_func!(ApproxPercentileCont, approx_percentile_cont_udaf); /// Computes the approximate percentile continuous of a set of numbers pub fn approx_percentile_cont( - expression: Expr, + within_group: Option<Vec<Sort>>, Review Comment: > Separately, why is this Option<Vec<Sort>>? As I understand it approx_percentile_cont must always have a WITHIN GROUP clause with a single ordering expression, so I would expect this to just be Sort to reflect that. I removed `Option`, because `Sort` is necessary for ordered set aggregate function as you mentioned. And also removed `Vec`. (cf. I initially define it as `Vec<Sort>`, because there are some aggregate functions which supports multiple ordering expression in `WITHIN GROUP` clause. However, I have made sure at this time that even those DB engine (ex. Oracle, SQL Server) uses only a single ordering expression in `percentile_cont` and `approx_percentile`.) Thank you for your guidance. -- 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
