alamb commented on code in PR #13393: URL: https://github.com/apache/datafusion/pull/13393#discussion_r1840688735
########## datafusion/physical-plan/src/windows/mod.rs: ########## @@ -120,7 +117,6 @@ pub fn create_window_expr( aggregate, ) } - // TODO: Ordering not supported for Window UDFs yet Review Comment: ORDER BY is already supported as the `order_by` clause is passed to `window_expr_from_aggregate_expr` ########## datafusion/expr/src/expr.rs: ########## @@ -697,13 +694,13 @@ impl AggregateFunction { } } -/// WindowFunction +/// A function used as a SQL window function +/// +/// In SQL, you can use: +/// - Actual window functions ([`WindowUDF`]) +/// - Noraml aggregate functions ([`AggregateUDF`]) Review Comment: Fixed in [6b1a243](https://github.com/apache/datafusion/pull/13393/commits/6b1a2439d44464d7f49c0c9f4df4f5f67f172344) ########## datafusion/expr/src/udwf.rs: ########## @@ -39,8 +39,16 @@ use datafusion_functions_window_common::field::WindowUDFFieldArgs; use datafusion_functions_window_common::partition::PartitionEvaluatorArgs; use datafusion_physical_expr_common::physical_expr::PhysicalExpr; -/// Logical representation of a user-defined window function (UDWF) -/// A UDWF is different from a UDF in that it is stateful across batches. +/// Logical representation of a user-defined window function (UDWF). Review Comment: I also threw in some drive by documentation improvements to sweeten the deal 🍯 -- 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