From 1aa01a99e9287efd8b95b830e3ccaf2eeb3d28a1 Mon Sep 17 00:00:00 2001 From: dmitrybugakov Date: Thu, 25 Apr 2024 23:14:58 +0200 Subject: [PATCH] [MINOR] Replace null treatment with boolean for #10230 --- datafusion/core/src/physical_planner.rs | 11 +++----- datafusion/expr/src/expr.rs | 22 ++++++++-------- datafusion/expr/src/expr_fn.rs | 26 +++++++++---------- datafusion/expr/src/tree_node.rs | 6 ++--- datafusion/expr/src/udaf.rs | 2 +- .../functions-aggregate/src/first_last.rs | 1 - datafusion/functions-aggregate/src/macros.rs | 4 +-- .../optimizer/src/analyzer/type_coercion.rs | 18 ++++++------- .../optimizer/src/common_subexpr_eliminate.rs | 2 +- .../optimizer/src/optimize_projections.rs | 2 +- .../src/replace_distinct_aggregate.rs | 2 +- .../src/single_distinct_to_groupby.rs | 22 ++++++++-------- .../proto/src/logical_plan/from_proto.rs | 4 +-- datafusion/proto/src/logical_plan/to_proto.rs | 2 +- .../tests/cases/roundtrip_logical_plan.rs | 10 +++---- datafusion/sql/src/expr/function.rs | 12 +++++---- datafusion/sql/src/expr/mod.rs | 8 +++--- datafusion/sql/src/unparser/expr.rs | 4 +-- .../substrait/src/logical_plan/consumer.rs | 4 +-- .../substrait/src/logical_plan/producer.rs | 2 +- 20 files changed, 81 insertions(+), 83 deletions(-) diff --git a/datafusion/core/src/physical_planner.rs b/datafusion/core/src/physical_planner.rs index b7b6c20b19bbc..e9f59f9b5527a 100644 --- a/datafusion/core/src/physical_planner.rs +++ b/datafusion/core/src/physical_planner.rs @@ -261,7 +261,7 @@ fn create_physical_name(e: &Expr, is_first_expr: bool) -> Result { args, filter, order_by, - null_treatment: _, + ignore_nulls:_, }) => match func_def { AggregateFunctionDefinition::BuiltIn(..) => create_function_physical_name( func_def.name(), @@ -1890,7 +1890,7 @@ pub fn create_aggregate_expr_with_name_and_maybe_filter( args, filter, order_by, - null_treatment, + ignore_nulls, }) => { let args = create_physical_exprs(args, logical_input_schema, execution_props)?; @@ -1903,9 +1903,6 @@ pub fn create_aggregate_expr_with_name_and_maybe_filter( None => None, }; - let ignore_nulls = null_treatment - .unwrap_or(sqlparser::ast::NullTreatment::RespectNulls) - == NullTreatment::IgnoreNulls; let (agg_expr, filter, order_by) = match func_def { AggregateFunctionDefinition::BuiltIn(fun) => { let physical_sort_exprs = match order_by { @@ -1925,7 +1922,7 @@ pub fn create_aggregate_expr_with_name_and_maybe_filter( &ordering_reqs, physical_input_schema, name, - ignore_nulls, + *ignore_nulls, )?; (agg_expr, filter, physical_sort_exprs) } @@ -1948,7 +1945,7 @@ pub fn create_aggregate_expr_with_name_and_maybe_filter( &ordering_reqs, physical_input_schema, name, - ignore_nulls, + *ignore_nulls, )?; (agg_expr, filter, physical_sort_exprs) } diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs index 0d8e8d816b335..a774f620a6c81 100644 --- a/datafusion/expr/src/expr.rs +++ b/datafusion/expr/src/expr.rs @@ -587,7 +587,7 @@ pub struct AggregateFunction { pub filter: Option>, /// Optional ordering pub order_by: Option>, - pub null_treatment: Option, + pub ignore_nulls: bool, } impl AggregateFunction { @@ -597,7 +597,7 @@ impl AggregateFunction { distinct: bool, filter: Option>, order_by: Option>, - null_treatment: Option, + ignore_nulls: bool, ) -> Self { Self { func_def: AggregateFunctionDefinition::BuiltIn(fun), @@ -605,7 +605,7 @@ impl AggregateFunction { distinct, filter, order_by, - null_treatment, + ignore_nulls, } } @@ -616,7 +616,7 @@ impl AggregateFunction { distinct: bool, filter: Option>, order_by: Option>, - null_treatment: Option, + ignore_nulls: bool, ) -> Self { Self { func_def: AggregateFunctionDefinition::UDF(udf), @@ -624,7 +624,7 @@ impl AggregateFunction { distinct, filter, order_by, - null_treatment, + ignore_nulls, } } } @@ -1502,12 +1502,12 @@ impl fmt::Display for Expr { ref args, filter, order_by, - null_treatment, + ignore_nulls, .. }) => { fmt_function(f, func_def.name(), *distinct, args, true)?; - if let Some(nt) = null_treatment { - write!(f, " {}", nt)?; + if *ignore_nulls { + write!(f, " {}", ignore_nulls)?; } if let Some(fe) = filter { write!(f, " FILTER (WHERE {fe})")?; @@ -1842,7 +1842,7 @@ fn create_name(e: &Expr) -> Result { args, filter, order_by, - null_treatment, + ignore_nulls, }) => { let name = match func_def { AggregateFunctionDefinition::BuiltIn(..) @@ -1862,8 +1862,8 @@ fn create_name(e: &Expr) -> Result { if let Some(order_by) = order_by { info += &format!(" ORDER BY [{}]", expr_vec_fmt!(order_by)); }; - if let Some(nt) = null_treatment { - info += &format!(" {}", nt); + if *ignore_nulls { + info += &format!(" {}", ignore_nulls); } match func_def { AggregateFunctionDefinition::BuiltIn(..) diff --git a/datafusion/expr/src/expr_fn.rs b/datafusion/expr/src/expr_fn.rs index 1d976a12cc4ff..656f12b514424 100644 --- a/datafusion/expr/src/expr_fn.rs +++ b/datafusion/expr/src/expr_fn.rs @@ -151,7 +151,7 @@ pub fn min(expr: Expr) -> Expr { false, None, None, - None, + false, )) } @@ -163,7 +163,7 @@ pub fn max(expr: Expr) -> Expr { false, None, None, - None, + false, )) } @@ -175,7 +175,7 @@ pub fn sum(expr: Expr) -> Expr { false, None, None, - None, + false, )) } @@ -187,7 +187,7 @@ pub fn array_agg(expr: Expr) -> Expr { false, None, None, - None, + false, )) } @@ -199,7 +199,7 @@ pub fn avg(expr: Expr) -> Expr { false, None, None, - None, + false, )) } @@ -211,7 +211,7 @@ pub fn count(expr: Expr) -> Expr { false, None, None, - None, + false, )) } @@ -268,7 +268,7 @@ pub fn count_distinct(expr: Expr) -> Expr { true, None, None, - None, + false, )) } @@ -291,7 +291,7 @@ pub fn approx_distinct(expr: Expr) -> Expr { false, None, None, - None, + false, )) } @@ -303,7 +303,7 @@ pub fn median(expr: Expr) -> Expr { false, None, None, - None, + false, )) } @@ -315,7 +315,7 @@ pub fn approx_median(expr: Expr) -> Expr { false, None, None, - None, + false, )) } @@ -327,7 +327,7 @@ pub fn approx_percentile_cont(expr: Expr, percentile: Expr) -> Expr { false, None, None, - None, + false, )) } @@ -343,7 +343,7 @@ pub fn approx_percentile_cont_with_weight( false, None, None, - None, + false, )) } @@ -414,7 +414,7 @@ pub fn stddev(expr: Expr) -> Expr { false, None, None, - None, + false, )) } diff --git a/datafusion/expr/src/tree_node.rs b/datafusion/expr/src/tree_node.rs index 471ed0b975b02..e58e9528d5e66 100644 --- a/datafusion/expr/src/tree_node.rs +++ b/datafusion/expr/src/tree_node.rs @@ -321,7 +321,7 @@ impl TreeNode for Expr { distinct, filter, order_by, - null_treatment, + ignore_nulls, }) => map_until_stop_and_collect!( transform_vec(args, &mut f), filter, @@ -338,7 +338,7 @@ impl TreeNode for Expr { distinct, new_filter, new_order_by, - null_treatment, + ignore_nulls, ))) } AggregateFunctionDefinition::UDF(fun) => { @@ -348,7 +348,7 @@ impl TreeNode for Expr { false, new_filter, new_order_by, - null_treatment, + ignore_nulls, ))) } AggregateFunctionDefinition::Name(_) => { diff --git a/datafusion/expr/src/udaf.rs b/datafusion/expr/src/udaf.rs index 67c3b51ca3739..92af1326b7d4c 100644 --- a/datafusion/expr/src/udaf.rs +++ b/datafusion/expr/src/udaf.rs @@ -137,7 +137,7 @@ impl AggregateUDF { false, None, None, - None, + false, )) } diff --git a/datafusion/functions-aggregate/src/first_last.rs b/datafusion/functions-aggregate/src/first_last.rs index 8dc4cee87a3bd..d689686e1978f 100644 --- a/datafusion/functions-aggregate/src/first_last.rs +++ b/datafusion/functions-aggregate/src/first_last.rs @@ -39,7 +39,6 @@ use datafusion_physical_expr_common::expressions; use datafusion_physical_expr_common::physical_expr::PhysicalExpr; use datafusion_physical_expr_common::sort_expr::{LexOrdering, PhysicalSortExpr}; use datafusion_physical_expr_common::utils::reverse_order_bys; -use sqlparser::ast::NullTreatment; use std::any::Any; use std::fmt::Debug; use std::sync::Arc; diff --git a/datafusion/functions-aggregate/src/macros.rs b/datafusion/functions-aggregate/src/macros.rs index 04f9fecb8b195..951b4ba91d475 100644 --- a/datafusion/functions-aggregate/src/macros.rs +++ b/datafusion/functions-aggregate/src/macros.rs @@ -25,7 +25,7 @@ macro_rules! make_udaf_function { distinct: bool, filter: Option>, order_by: Option>, - null_treatment: Option + ignore_nulls: bool ) -> Expr { Expr::AggregateFunction(datafusion_expr::expr::AggregateFunction::new_udf( $AGGREGATE_UDF_FN(), @@ -33,7 +33,7 @@ macro_rules! make_udaf_function { distinct, filter, order_by, - null_treatment, + ignore_nulls, )) } diff --git a/datafusion/optimizer/src/analyzer/type_coercion.rs b/datafusion/optimizer/src/analyzer/type_coercion.rs index f96a359f9d47b..974f0dd17f3c6 100644 --- a/datafusion/optimizer/src/analyzer/type_coercion.rs +++ b/datafusion/optimizer/src/analyzer/type_coercion.rs @@ -328,7 +328,7 @@ impl TreeNodeRewriter for TypeCoercionRewriter { distinct, filter, order_by, - null_treatment, + ignore_nulls, }) => match func_def { AggregateFunctionDefinition::BuiltIn(fun) => { let new_expr = coerce_agg_exprs_for_signature( @@ -344,7 +344,7 @@ impl TreeNodeRewriter for TypeCoercionRewriter { distinct, filter, order_by, - null_treatment, + ignore_nulls, ), ))) } @@ -361,7 +361,7 @@ impl TreeNodeRewriter for TypeCoercionRewriter { false, filter, order_by, - null_treatment, + ignore_nulls, ), ))) } @@ -890,7 +890,7 @@ mod test { false, None, None, - None, + false, )); let plan = LogicalPlan::Projection(Projection::try_new(vec![udaf], empty)?); let expected = "Projection: MY_AVG(CAST(Int64(10) AS Float64))\n EmptyRelation"; @@ -919,7 +919,7 @@ mod test { false, None, None, - None, + false, )); let plan = LogicalPlan::Projection(Projection::try_new(vec![udaf], empty)?); let err = assert_analyzed_plan_eq(Arc::new(TypeCoercion::new()), &plan, "") @@ -942,7 +942,7 @@ mod test { false, None, None, - None, + false, )); let plan = LogicalPlan::Projection(Projection::try_new(vec![agg_expr], empty)?); let expected = "Projection: AVG(CAST(Int64(12) AS Float64))\n EmptyRelation"; @@ -956,7 +956,7 @@ mod test { false, None, None, - None, + false, )); let plan = LogicalPlan::Projection(Projection::try_new(vec![agg_expr], empty)?); let expected = "Projection: AVG(CAST(a AS Float64))\n EmptyRelation"; @@ -974,7 +974,7 @@ mod test { false, None, None, - None, + false, )); let err = Projection::try_new(vec![agg_expr], empty) .err() @@ -997,7 +997,7 @@ mod test { false, None, None, - None, + false, )); let err = Projection::try_new(vec![agg_expr], empty) diff --git a/datafusion/optimizer/src/common_subexpr_eliminate.rs b/datafusion/optimizer/src/common_subexpr_eliminate.rs index cb3b4accf35d0..38d55dc2c681e 100644 --- a/datafusion/optimizer/src/common_subexpr_eliminate.rs +++ b/datafusion/optimizer/src/common_subexpr_eliminate.rs @@ -839,7 +839,7 @@ mod test { false, None, None, - None, + false, )) }; diff --git a/datafusion/optimizer/src/optimize_projections.rs b/datafusion/optimizer/src/optimize_projections.rs index 70ffd8f244984..4879acce609aa 100644 --- a/datafusion/optimizer/src/optimize_projections.rs +++ b/datafusion/optimizer/src/optimize_projections.rs @@ -2011,7 +2011,7 @@ mod tests { false, Some(Box::new(col("c").gt(lit(42)))), None, - None, + false, )); let plan = LogicalPlanBuilder::from(table_scan) diff --git a/datafusion/optimizer/src/replace_distinct_aggregate.rs b/datafusion/optimizer/src/replace_distinct_aggregate.rs index 4f68e2623f403..5aca080029b6d 100644 --- a/datafusion/optimizer/src/replace_distinct_aggregate.rs +++ b/datafusion/optimizer/src/replace_distinct_aggregate.rs @@ -98,7 +98,7 @@ impl OptimizerRule for ReplaceDistinctWithAggregate { false, None, sort_expr.clone(), - None, + false, )) }) .collect::>(); diff --git a/datafusion/optimizer/src/single_distinct_to_groupby.rs b/datafusion/optimizer/src/single_distinct_to_groupby.rs index aaf4667fb0005..c34973fb1f30f 100644 --- a/datafusion/optimizer/src/single_distinct_to_groupby.rs +++ b/datafusion/optimizer/src/single_distinct_to_groupby.rs @@ -76,7 +76,7 @@ fn is_single_distinct_agg(plan: &LogicalPlan) -> Result { args, filter, order_by, - null_treatment: _, + ignore_nulls:_, }) = expr { if filter.is_some() || order_by.is_some() { @@ -200,7 +200,7 @@ impl OptimizerRule for SingleDistinctToGroupBy { false, None, None, - None, + false, )) .alias(&alias_str), ); @@ -210,7 +210,7 @@ impl OptimizerRule for SingleDistinctToGroupBy { false, None, None, - None, + false, ))) } else { Ok(Expr::AggregateFunction(AggregateFunction::new( @@ -219,7 +219,7 @@ impl OptimizerRule for SingleDistinctToGroupBy { false, // intentional to remove distinct here None, None, - None, + false, ))) } } @@ -479,7 +479,7 @@ mod tests { true, None, None, - None, + false, )), ], )? @@ -544,7 +544,7 @@ mod tests { true, None, None, - None, + false, )), ], )? @@ -607,7 +607,7 @@ mod tests { false, Some(Box::new(col("a").gt(lit(5)))), None, - None, + false, )); let plan = LogicalPlanBuilder::from(table_scan) .aggregate(vec![col("c")], vec![expr, count_distinct(col("b"))])? @@ -630,7 +630,7 @@ mod tests { true, Some(Box::new(col("a").gt(lit(5)))), None, - None, + false, )); let plan = LogicalPlanBuilder::from(table_scan) .aggregate(vec![col("c")], vec![sum(col("a")), expr])? @@ -653,7 +653,7 @@ mod tests { false, None, Some(vec![col("a")]), - None, + false, )); let plan = LogicalPlanBuilder::from(table_scan) .aggregate(vec![col("c")], vec![expr, count_distinct(col("b"))])? @@ -676,7 +676,7 @@ mod tests { true, None, Some(vec![col("a")]), - None, + false, )); let plan = LogicalPlanBuilder::from(table_scan) .aggregate(vec![col("c")], vec![sum(col("a")), expr])? @@ -699,7 +699,7 @@ mod tests { true, Some(Box::new(col("a").gt(lit(5)))), Some(vec![col("a")]), - None, + false, )); let plan = LogicalPlanBuilder::from(table_scan) .aggregate(vec![col("c")], vec![sum(col("a")), expr])? diff --git a/datafusion/proto/src/logical_plan/from_proto.rs b/datafusion/proto/src/logical_plan/from_proto.rs index 83b232da9d21c..65cf78e5e24f8 100644 --- a/datafusion/proto/src/logical_plan/from_proto.rs +++ b/datafusion/proto/src/logical_plan/from_proto.rs @@ -1048,7 +1048,7 @@ pub fn parse_expr( parse_optional_expr(expr.filter.as_deref(), registry, codec)? .map(Box::new), parse_vec_expr(&expr.order_by, registry, codec)?, - None, + false, ))) } ExprType::Alias(alias) => Ok(Expr::Alias(Alias::new( @@ -1287,7 +1287,7 @@ pub fn parse_expr( false, parse_optional_expr(pb.filter.as_deref(), registry, codec)?.map(Box::new), parse_vec_expr(&pb.order_by, registry, codec)?, - None, + false, ))) } diff --git a/datafusion/proto/src/logical_plan/to_proto.rs b/datafusion/proto/src/logical_plan/to_proto.rs index 45aebc88dc639..6ea160680c15f 100644 --- a/datafusion/proto/src/logical_plan/to_proto.rs +++ b/datafusion/proto/src/logical_plan/to_proto.rs @@ -646,7 +646,7 @@ pub fn serialize_expr( ref distinct, ref filter, ref order_by, - null_treatment: _, + ignore_nulls:_, }) => match func_def { AggregateFunctionDefinition::BuiltIn(fun) => { let aggr_function = match fun { diff --git a/datafusion/proto/tests/cases/roundtrip_logical_plan.rs b/datafusion/proto/tests/cases/roundtrip_logical_plan.rs index 1fd6160c2c6c1..de1e9665eb97b 100644 --- a/datafusion/proto/tests/cases/roundtrip_logical_plan.rs +++ b/datafusion/proto/tests/cases/roundtrip_logical_plan.rs @@ -613,7 +613,7 @@ async fn roundtrip_expr_api() -> Result<()> { lit(1), ), array_replace_all(make_array(vec![lit(1), lit(2), lit(3)]), lit(2), lit(4)), - first_value(vec![lit(1)], false, None, None, None), + first_value(vec![lit(1)], false, None, None, false), ]; // ensure expressions created with the expr api can be round tripped @@ -1689,7 +1689,7 @@ fn roundtrip_count() { false, None, None, - None, + false, )); let ctx = SessionContext::new(); roundtrip_expr_test(test_expr, ctx); @@ -1703,7 +1703,7 @@ fn roundtrip_count_distinct() { true, None, None, - None, + false, )); let ctx = SessionContext::new(); roundtrip_expr_test(test_expr, ctx); @@ -1717,7 +1717,7 @@ fn roundtrip_approx_percentile_cont() { false, None, None, - None, + false, )); let ctx = SessionContext::new(); @@ -1774,7 +1774,7 @@ fn roundtrip_aggregate_udf() { false, Some(Box::new(lit(true))), None, - None, + false, )); let ctx = SessionContext::new(); diff --git a/datafusion/sql/src/expr/function.rs b/datafusion/sql/src/expr/function.rs index 68cba15634d58..0243bece81adb 100644 --- a/datafusion/sql/src/expr/function.rs +++ b/datafusion/sql/src/expr/function.rs @@ -29,9 +29,7 @@ use datafusion_expr::{ expr::{ScalarFunction, Unnest}, BuiltInWindowFunction, }; -use sqlparser::ast::{ - Expr as SQLExpr, Function as SQLFunction, FunctionArg, FunctionArgExpr, WindowType, -}; +use sqlparser::ast::{Expr as SQLExpr, Function as SQLFunction, FunctionArg, FunctionArgExpr, NullTreatment, WindowType}; use std::str::FromStr; use strum::IntoEnumIterator; @@ -102,6 +100,10 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { // required ordering should be defined in OVER clause. let is_function_window = over.is_some(); + let ignore_nulls = null_treatment + .unwrap_or(sqlparser::ast::NullTreatment::RespectNulls) + == NullTreatment::IgnoreNulls; + let name = if name.0.len() > 1 { // DF doesn't handle compound identifiers // (e.g. "foo.bar") for function names yet @@ -230,7 +232,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { false, None, order_by, - null_treatment, + ignore_nulls, ))); } @@ -251,7 +253,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { distinct, filter, order_by, - null_treatment, + ignore_nulls, ))); }; } diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs index 0d1db8a29cceb..61197261f8f87 100644 --- a/datafusion/sql/src/expr/mod.rs +++ b/datafusion/sql/src/expr/mod.rs @@ -221,7 +221,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { agg_func.distinct, agg_func.filter.clone(), agg_func.order_by.clone(), - agg_func.null_treatment, + agg_func.ignore_nulls, )), true) }, _ => (expr, false), @@ -722,7 +722,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { distinct, None, order_by, - None, + false, ))) // see if we can rewrite it into NTH-VALUE } @@ -907,7 +907,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { args, distinct, order_by, - null_treatment, + ignore_nulls, filter: None, // filter is passed in }) => Ok(Expr::AggregateFunction(expr::AggregateFunction::new( fun, @@ -919,7 +919,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { planner_context, )?)), order_by, - null_treatment, + ignore_nulls, ))), Expr::AggregateFunction(..) => { internal_err!("Expected null filter clause in aggregate function") diff --git a/datafusion/sql/src/unparser/expr.rs b/datafusion/sql/src/unparser/expr.rs index d091fbe14dbd5..d3352faf1e6a3 100644 --- a/datafusion/sql/src/unparser/expr.rs +++ b/datafusion/sql/src/unparser/expr.rs @@ -884,7 +884,7 @@ mod tests { distinct: false, filter: None, order_by: None, - null_treatment: None, + ignore_nulls: false, }), r#"SUM("a")"#, ), @@ -897,7 +897,7 @@ mod tests { distinct: true, filter: None, order_by: None, - null_treatment: None, + ignore_nulls: false, }), "COUNT(DISTINCT *)", ), diff --git a/datafusion/substrait/src/logical_plan/consumer.rs b/datafusion/substrait/src/logical_plan/consumer.rs index fab4528c0b421..1ec43973cf3cc 100644 --- a/datafusion/substrait/src/logical_plan/consumer.rs +++ b/datafusion/substrait/src/logical_plan/consumer.rs @@ -749,12 +749,12 @@ pub async fn from_substrait_agg_func( // try udaf first, then built-in aggr fn. if let Ok(fun) = ctx.udaf(function_name) { Ok(Arc::new(Expr::AggregateFunction( - expr::AggregateFunction::new_udf(fun, args, distinct, filter, order_by, None), + expr::AggregateFunction::new_udf(fun, args, distinct, filter, order_by, false), ))) } else if let Ok(fun) = aggregate_function::AggregateFunction::from_str(function_name) { Ok(Arc::new(Expr::AggregateFunction( - expr::AggregateFunction::new(fun, args, distinct, filter, order_by, None), + expr::AggregateFunction::new(fun, args, distinct, filter, order_by, false), ))) } else { not_impl_err!( diff --git a/datafusion/substrait/src/logical_plan/producer.rs b/datafusion/substrait/src/logical_plan/producer.rs index a6a38ab6145c0..1a6ef7f079253 100644 --- a/datafusion/substrait/src/logical_plan/producer.rs +++ b/datafusion/substrait/src/logical_plan/producer.rs @@ -672,7 +672,7 @@ pub fn to_substrait_agg_measure( ), ) -> Result { match expr { - Expr::AggregateFunction(expr::AggregateFunction { func_def, args, distinct, filter, order_by, null_treatment: _, }) => { + Expr::AggregateFunction(expr::AggregateFunction { func_def, args, distinct, filter, order_by, ignore_nulls:_, }) => { match func_def { AggregateFunctionDefinition::BuiltIn (fun) => { let sorts = if let Some(order_by) = order_by {