alamb commented on code in PR #13536: URL: https://github.com/apache/datafusion/pull/13536#discussion_r1855181320
########## datafusion/physical-expr/src/window/mod.rs: ########## @@ -16,15 +16,15 @@ // under the License. mod aggregate; -mod built_in; -mod built_in_window_function_expr; mod sliding_aggregate; +mod udf; Review Comment: It is somewhat strange that we now have "UD ..." for user defined window functions as everything is a user defined window function. Maybe we could use the names `StandardWindowExpr` and `StandardWindowFunctionExpr` instead I think `WindowExpr` can be either an aggregate (like `sum`) or one of these standard window exprs (first_value, etc) ########## datafusion/physical-expr/src/window/mod.rs: ########## @@ -16,15 +16,15 @@ // under the License. mod aggregate; -mod built_in; -mod built_in_window_function_expr; mod sliding_aggregate; +mod udf; +mod udf_window_function_expr; mod window_expr; pub use aggregate::PlainAggregateWindowExpr; -pub use built_in::BuiltInWindowExpr; -pub use built_in_window_function_expr::BuiltInWindowFunctionExpr; pub use sliding_aggregate::SlidingAggregateWindowExpr; +pub use udf::UDFWindowExpr; Review Comment: Could you please add a "backwards compatible typedef" here to help people upgrading who might have code that refers to `BuiltInWindowExpr` and BuiltInWindowFunctionExpr? For example https://github.com/apache/datafusion/blob/6a02384d198b3420b96892cc7a72fc202770375c/datafusion/physical-expr-common/src/sort_expr.rs#L505-L506 Something like ```rust #[deprecated(since = "44.0.0", note = "use UDFWindowExpr")] pub type BuiltInWindowExpr = UDFWindowExpr; #[deprecated(since = "44.0.0", note = "use UDFWindowFunctionExpr")] pub type BuiltInWindowFunctionExpr = UDFWindowFunctionExpr; ``` -- 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]
