Skip to content

Commit

Permalink
[MINOR] Replace null treatment with boolean for apache#10230
Browse files Browse the repository at this point in the history
  • Loading branch information
@demetribu
demetribu committed Apr 25, 2024
1 parent b87f210 commit 782b859
Show file tree
Hide file tree
Showing 20 changed files with 85 additions and 83 deletions.
15 changes: 8 additions & 7 deletions datafusion/core/src/physical_planner.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ fn create_physical_name(e: &Expr, is_first_expr: bool) -> Result<String> {
args,
filter,
order_by,
null_treatment: _,
ignore_nulls:_,
}) => match func_def {
AggregateFunctionDefinition::BuiltIn(..) => create_function_physical_name(
func_def.name(),
Expand Down Expand Up @@ -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)?;
Expand All @@ -1903,9 +1903,10 @@ 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 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 {
Expand All @@ -1925,7 +1926,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)
}
Expand All @@ -1948,7 +1949,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)
}
Expand Down
22 changes: 11 additions & 11 deletions datafusion/expr/src/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ pub struct AggregateFunction {
pub filter: Option<Box<Expr>>,
/// Optional ordering
pub order_by: Option<Vec<Expr>>,
pub null_treatment: Option<NullTreatment>,
pub ignore_nulls: bool,
}

impl AggregateFunction {
Expand All @@ -597,15 +597,15 @@ impl AggregateFunction {
distinct: bool,
filter: Option<Box<Expr>>,
order_by: Option<Vec<Expr>>,
null_treatment: Option<NullTreatment>,
ignore_nulls: bool,
) -> Self {
Self {
func_def: AggregateFunctionDefinition::BuiltIn(fun),
args,
distinct,
filter,
order_by,
null_treatment,
ignore_nulls,
}
}

Expand All @@ -616,15 +616,15 @@ impl AggregateFunction {
distinct: bool,
filter: Option<Box<Expr>>,
order_by: Option<Vec<Expr>>,
null_treatment: Option<NullTreatment>,
ignore_nulls: bool,
) -> Self {
Self {
func_def: AggregateFunctionDefinition::UDF(udf),
args,
distinct,
filter,
order_by,
null_treatment,
ignore_nulls,
}
}
}
Expand Down Expand Up @@ -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})")?;
Expand Down Expand Up @@ -1842,7 +1842,7 @@ fn create_name(e: &Expr) -> Result<String> {
args,
filter,
order_by,
null_treatment,
ignore_nulls,
}) => {
let name = match func_def {
AggregateFunctionDefinition::BuiltIn(..)
Expand All @@ -1862,8 +1862,8 @@ fn create_name(e: &Expr) -> Result<String> {
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(..)
Expand Down
26 changes: 13 additions & 13 deletions datafusion/expr/src/expr_fn.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ pub fn min(expr: Expr) -> Expr {
false,
None,
None,
None,
false,
))
}

Expand All @@ -163,7 +163,7 @@ pub fn max(expr: Expr) -> Expr {
false,
None,
None,
None,
false,
))
}

Expand All @@ -175,7 +175,7 @@ pub fn sum(expr: Expr) -> Expr {
false,
None,
None,
None,
false,
))
}

Expand All @@ -187,7 +187,7 @@ pub fn array_agg(expr: Expr) -> Expr {
false,
None,
None,
None,
false,
))
}

Expand All @@ -199,7 +199,7 @@ pub fn avg(expr: Expr) -> Expr {
false,
None,
None,
None,
false,
))
}

Expand All @@ -211,7 +211,7 @@ pub fn count(expr: Expr) -> Expr {
false,
None,
None,
None,
false,
))
}

Expand Down Expand Up @@ -268,7 +268,7 @@ pub fn count_distinct(expr: Expr) -> Expr {
true,
None,
None,
None,
false,
))
}

Expand All @@ -291,7 +291,7 @@ pub fn approx_distinct(expr: Expr) -> Expr {
false,
None,
None,
None,
false,
))
}

Expand All @@ -303,7 +303,7 @@ pub fn median(expr: Expr) -> Expr {
false,
None,
None,
None,
false,
))
}

Expand All @@ -315,7 +315,7 @@ pub fn approx_median(expr: Expr) -> Expr {
false,
None,
None,
None,
false,
))
}

Expand All @@ -327,7 +327,7 @@ pub fn approx_percentile_cont(expr: Expr, percentile: Expr) -> Expr {
false,
None,
None,
None,
false,
))
}

Expand All @@ -343,7 +343,7 @@ pub fn approx_percentile_cont_with_weight(
false,
None,
None,
None,
false,
))
}

Expand Down Expand Up @@ -414,7 +414,7 @@ pub fn stddev(expr: Expr) -> Expr {
false,
None,
None,
None,
false,
))
}

Expand Down
6 changes: 3 additions & 3 deletions datafusion/expr/src/tree_node.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -338,7 +338,7 @@ impl TreeNode for Expr {
distinct,
new_filter,
new_order_by,
null_treatment,
ignore_nulls,
)))
}
AggregateFunctionDefinition::UDF(fun) => {
Expand All @@ -348,7 +348,7 @@ impl TreeNode for Expr {
false,
new_filter,
new_order_by,
null_treatment,
ignore_nulls,
)))
}
AggregateFunctionDefinition::Name(_) => {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/expr/src/udaf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ impl AggregateUDF {
false,
None,
None,
None,
false,
))
}

Expand Down
1 change: 0 additions & 1 deletion datafusion/functions-aggregate/src/first_last.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions datafusion/functions-aggregate/src/macros.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ macro_rules! make_udaf_function {
distinct: bool,
filter: Option<Box<Expr>>,
order_by: Option<Vec<Expr>>,
null_treatment: Option<NullTreatment>
ignore_nulls: bool
) -> Expr {
Expr::AggregateFunction(datafusion_expr::expr::AggregateFunction::new_udf(
$AGGREGATE_UDF_FN(),
args,
distinct,
filter,
order_by,
null_treatment,
ignore_nulls,
))
}

Expand Down
18 changes: 9 additions & 9 deletions datafusion/optimizer/src/analyzer/type_coercion.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -344,7 +344,7 @@ impl TreeNodeRewriter for TypeCoercionRewriter {
distinct,
filter,
order_by,
null_treatment,
ignore_nulls,
),
)))
}
Expand All @@ -361,7 +361,7 @@ impl TreeNodeRewriter for TypeCoercionRewriter {
false,
filter,
order_by,
null_treatment,
ignore_nulls,
),
)))
}
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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, "")
Expand All @@ -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";
Expand All @@ -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";
Expand All @@ -974,7 +974,7 @@ mod test {
false,
None,
None,
None,
false,
));
let err = Projection::try_new(vec![agg_expr], empty)
.err()
Expand All @@ -997,7 +997,7 @@ mod test {
false,
None,
None,
None,
false,
));

let err = Projection::try_new(vec![agg_expr], empty)
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/common_subexpr_eliminate.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,7 @@ mod test {
false,
None,
None,
None,
false,
))
};

Expand Down
Loading

0 comments on commit 782b859

Please sign in to comment.