timsaucer commented on PR #11550: URL: https://github.com/apache/datafusion/pull/11550#issuecomment-2245192874
Question: it looks like we have some functions that have both aggregate functions and window functions defined. Specifically looking at `first_value` and `last_value`. My thought is that the current PR just keeps with the existing pattern but I think we should merge these two, right? Should all aggregate functions be viable window functions? Right now I have `datafusion_expr::window_function::first_value()` will return a window function expression of type `BuiltInWindowFunction::FirstValue` and `datafusion_functions_aggregate::first_last::first_value()` will return an aggregate expression of type `AggregateUDF` and `datafusion_functions_aggregate::first_last::FirstValue`. My thinking is that we should stick with this PR for now but open an issue to converge all of these. I would expect the following psuedocode to be equivalent ``` let window_expr = datafusion_expr::window_function::first_value(col("a")); let agg_expr = datafusion_functions_aggregate::first_last::first_value(col("a")); ``` Please correct me if I'm missing something here. -- 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