alamb commented on code in PR #10560: URL: https://github.com/apache/datafusion/pull/10560#discussion_r1626296887
########## datafusion-examples/examples/advanced_udaf.rs: ########## @@ -412,7 +412,7 @@ async fn main() -> Result<()> { let df = ctx.table("t").await?; // perform the aggregation - let df = df.aggregate(vec![], vec![geometric_mean.call(vec![col("a")])])?; + let df = df.aggregate(vec![], vec![geometric_mean.call(vec![col("a")]).build()])?; Review Comment: Sorry for the delay in reviewing this @jayzhan211 -- I have been thinking about this PR and I am struggling On one hand, having a builder allows for exposing the ability to provide other types of arguments to aggregate functions (e.g. `ORDER BY` or `FILTER`) On the other hand, I feel like this API change (requiring a call to `build()` after `geometric_mean.call` makes DataFusion harder to learn / use as most calls to aggregate functions are made with just the arguments. I was thinking about what an ideal user experience would be. What do you think about something that permits using the existing `expr_fn` API, but also allows easier construction of more advanced aggregate calls What about something like this (as today, no need to call build) ```rust // geometric_mean(a) let agg = geometric_mean.call(vec![col("a")]); let df = df.aggregate(vec![], vec![agg])?; ``` And then then to create an `ORDER BY` the user could create and use a builder like this: ```rust // geometric_mean(a FILTER b > c) let agg = geometric_mean.call(vec![col("a")]) .agg_builder() // creates a builder that wraps an Expr, no error return .filter(col("b").gt(col("c"(( .build()?; // check here if everything was ok let df = df.aggregate(vec![], vec![agg])?; ``` We could probably implement a trait for `Expr` to add `agg_builder` (or maybe just add it directly) -- 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