jayzhan211 commented on PR #10560:
URL: https://github.com/apache/datafusion/pull/10560#issuecomment-2126011843
> Thank you @jayzhan211 -- this is looking really cool. I have some feeback
on the API design
>
> One major thing I think that might be worth considering is now to get one
of these builders. Right now they are implemented as `count_builder` free
functions, which means both adding many new functions as well as that they
won't work for user defined aggregate functions
>
> I wonder if it would be possible to add something to the `AggregateUDF`
instead maybe make an extension trait so people could make a builder for any
arbitrary aggregate expression,
>
> So today to call a UDF you do
>
> ```rust
> // Given an aggregate UDF:
> let agg_udf: AggregateUdf = todo!();
> // agg(a)
> let expr = agg_udf.call(col("a"))
> ```
>
> Could we implement a builder that works like
>
> ```rust
> // agg(a ORDER BY b)
> let expr = agg_udf.builder()
> .args(col("a"))
> .order_by("b")
> .build()
> ```
>
> Or I potentailly keep the "call" syntax:
>
> ```rust
> // agg(a ORDER BY b)
> let expr = agg_udf.builder()
> .order_by(col("b"))
> .call(col("a"))
> .build()
> ```
>
> What do you think?
> which means both adding many new functions as well as that they won't work
for user defined aggregate functions
I think they can build the function with macro in function-aggregate, so
they can have their function with ExprBuilder extension. But, then I also need
to move the macro to `datafusion-expr` 🤔
--
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
