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: [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]
