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

Reply via email to