From da30827d19992933ea3bda3ae44f89762b9bd76d Mon Sep 17 00:00:00 2001 From: jayzhan211 Date: Tue, 20 Aug 2024 15:20:41 +0800 Subject: [PATCH] cleanup Signed-off-by: jayzhan211 --- datafusion/expr/src/expr_schema.rs | 18 ++++-------------- datafusion/expr/src/udaf.rs | 6 +++--- datafusion/functions-aggregate/src/count.rs | 3 +-- .../src/aggregate.rs | 7 +------ 4 files changed, 9 insertions(+), 25 deletions(-) diff --git a/datafusion/expr/src/expr_schema.rs b/datafusion/expr/src/expr_schema.rs index f1b47a0a6af2..10ec10e61239 100644 --- a/datafusion/expr/src/expr_schema.rs +++ b/datafusion/expr/src/expr_schema.rs @@ -338,14 +338,10 @@ impl ExprSchemable for Expr { Expr::ScalarFunction(ScalarFunction { func, args }) => { Ok(func.is_nullable(args, input_schema)) } - Expr::AggregateFunction(AggregateFunction { func, args, .. }) => { - let nullables = args - .iter() - .map(|e| e.nullable(input_schema)) - .collect::>>()?; - Ok(func.is_nullable(&nullables)) + Expr::AggregateFunction(AggregateFunction { func, .. }) => { + Ok(func.is_nullable()) } - Expr::WindowFunction(WindowFunction { fun, args, .. }) => match fun { + Expr::WindowFunction(WindowFunction { fun, .. }) => match fun { WindowFunctionDefinition::BuiltInWindowFunction(func) => { if func.name() == "RANK" || func.name() == "NTILE" @@ -356,13 +352,7 @@ impl ExprSchemable for Expr { Ok(true) } } - WindowFunctionDefinition::AggregateUDF(func) => { - let nullables = args - .iter() - .map(|e| e.nullable(input_schema)) - .collect::>>()?; - Ok(func.is_nullable(&nullables)) - } + WindowFunctionDefinition::AggregateUDF(func) => Ok(func.is_nullable()), WindowFunctionDefinition::WindowUDF(udwf) => Ok(udwf.nullable()), }, Expr::ScalarVariable(_, _) diff --git a/datafusion/expr/src/udaf.rs b/datafusion/expr/src/udaf.rs index 1afcad4b243e..241b6cc0d62d 100644 --- a/datafusion/expr/src/udaf.rs +++ b/datafusion/expr/src/udaf.rs @@ -163,8 +163,8 @@ impl AggregateUDF { self.inner.name() } - pub fn is_nullable(&self, nullables: &[bool]) -> bool { - self.inner.is_nullable(nullables) + pub fn is_nullable(&self) -> bool { + self.inner.is_nullable() } /// Returns the aliases for this function. @@ -356,7 +356,7 @@ pub trait AggregateUDFImpl: Debug + Send + Sync { /// Nullable means that that the function could return `null` for any inputs. /// For example, aggregate functions like `COUNT` always return a non null value /// but others like `MIN` will return `NULL` if there is nullable input. - fn is_nullable(&self, _nullables: &[bool]) -> bool { + fn is_nullable(&self) -> bool { true } diff --git a/datafusion/functions-aggregate/src/count.rs b/datafusion/functions-aggregate/src/count.rs index 591f8f226959..417e28e72a71 100644 --- a/datafusion/functions-aggregate/src/count.rs +++ b/datafusion/functions-aggregate/src/count.rs @@ -121,8 +121,7 @@ impl AggregateUDFImpl for Count { Ok(DataType::Int64) } - // Count is always nullable regardless of the input nullability - fn is_nullable(&self, _nullables: &[bool]) -> bool { + fn is_nullable(&self) -> bool { false } diff --git a/datafusion/physical-expr-functions-aggregate/src/aggregate.rs b/datafusion/physical-expr-functions-aggregate/src/aggregate.rs index ad9f53c9c77f..aa1d1999a339 100644 --- a/datafusion/physical-expr-functions-aggregate/src/aggregate.rs +++ b/datafusion/physical-expr-functions-aggregate/src/aggregate.rs @@ -103,11 +103,6 @@ impl AggregateExprBuilder { .map(|arg| arg.data_type(&schema)) .collect::>>()?; - let input_nullables = args - .iter() - .map(|arg| arg.nullable(&schema)) - .collect::>>()?; - check_arg_count( fun.name(), &input_exprs_types, @@ -115,7 +110,7 @@ impl AggregateExprBuilder { )?; let data_type = fun.return_type(&input_exprs_types)?; - let is_nullable = fun.is_nullable(&input_nullables); + let is_nullable = fun.is_nullable(); let name = match alias { // TODO: Ideally, we should build the name from physical expressions None => create_function_physical_name(fun.name(), is_distinct, &[], None)?,