alamb commented on code in PR #11550: URL: https://github.com/apache/datafusion/pull/11550#discussion_r1686922713
########## datafusion/expr/src/expr_fn.rs: ########## @@ -676,6 +679,266 @@ pub fn interval_month_day_nano_lit(value: &str) -> Expr { Expr::Literal(ScalarValue::IntervalMonthDayNano(interval)) } +/// Extensions for configuring [`Expr::AggregateFunction`] +/// +/// Adds methods to [`Expr`] that make it easy to set optional aggregate options +/// such as `ORDER BY`, `FILTER` and `DISTINCT` +/// +/// # Example +/// ```no_run +/// # use datafusion_common::Result; +/// # use datafusion_expr::{AggregateUDF, col, Expr, lit}; +/// # use sqlparser::ast::NullTreatment; +/// # fn count(arg: Expr) -> Expr { todo!{} } +/// # fn first_value(arg: Expr) -> Expr { todo!{} } +/// # fn main() -> Result<()> { +/// use datafusion_expr::ExprFunctionExt; +/// +/// // Create COUNT(x FILTER y > 5) +/// let agg = count(col("x")) +/// .filter(col("y").gt(lit(5))) +/// .build()?; +/// // Create FIRST_VALUE(x ORDER BY y IGNORE NULLS) +/// let sort_expr = col("y").sort(true, true); +/// let agg = first_value(col("x")) +/// .order_by(vec![sort_expr]) +/// .null_treatment(NullTreatment::IgnoreNulls) +/// .build()?; +/// # Ok(()) +/// # } +/// ``` +pub trait ExprFunctionExt { Review Comment: > The one problem I see there is that we would end up with Expr implementing two traits with identical function calls, which would be less user friendly. I agree that is not a good user experience > then this PR is ready to go assuming we come to agreement on if this should be a single trait or broken into two for Aggregate vs Window. I wonder if we could do something like this: ```rust trait WindowExt: AggregateExt { ... } ``` which is kind of like Rust's inheritance -- it basically says that anything that implements `WindowExt` also implements `AggregateExt` And then the builder could be extended for window specific functions (and perhaps `build` made to error if the underlying aggregate function couldn't be used as a window function) -- 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