jayzhan211 commented on code in PR #11550:
URL: https://github.com/apache/datafusion/pull/11550#discussion_r1688132417
##########
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 {
+ /// Add `ORDER BY <order_by>`
+ ///
+ /// Note: `order_by` must be [`Expr::Sort`]
+ fn order_by(self, order_by: Vec<Expr>) -> ExprFuncBuilder;
+ /// Add `FILTER <filter>`
+ fn filter(self, filter: Expr) -> ExprFuncBuilder;
+ /// Add `DISTINCT`
+ fn distinct(self) -> ExprFuncBuilder;
+ /// Add `RESPECT NULLS` or `IGNORE NULLS`
+ fn null_treatment(
+ self,
+ null_treatment: impl Into<Option<NullTreatment>>,
+ ) -> ExprFuncBuilder;
+ // Add `PARTITION BY`
+ fn partition_by(self, partition_by: Vec<Expr>) -> ExprFuncBuilder;
+ // Add appropriate window frame conditions
+ fn window_frame(self, window_frame: WindowFrame) -> ExprFuncBuilder;
+}
+
+#[derive(Debug, Clone)]
+pub enum ExprFuncKind {
+ Aggregate(AggregateFunction),
+ Window(WindowFunction),
+}
+
+/// Implementation of [`ExprFunctionExt`].
+///
+/// See [`ExprFunctionExt`] for usage and examples
+#[derive(Debug, Clone)]
+pub struct ExprFuncBuilder {
+ fun: Option<ExprFuncKind>,
Review Comment:
> Since we are sticking with the builder approach, don't we need this in
case someone attempts to create a builder from an Expr that is not aggregate or
window?
>
> ```
> col("a").order_by(vec![col("b")])
> ```
>
> This is a wrong use but won't be caught until we call `.build()` so up to
that point don't we need the `fun` field to be None?
Makes sense, it is for incorrect usage
--
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
