Skip to content

Commit

Permalink
Minor: Remove redundant BuiltinScalarFunction::supports_zero_argument…
Browse files Browse the repository at this point in the history
…() (#8059)

* deprecate BuiltinScalarFunction::supports_zero_argument()

* unify old supports_zero_argument() impl
  • Loading branch information
2010YOUY01 authored Nov 6, 2023
1 parent af3ce6b commit 308c354
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 18 deletions.
23 changes: 11 additions & 12 deletions datafusion/expr/src/built_in_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,18 +325,14 @@ fn function_to_name() -> &'static HashMap<BuiltinScalarFunction, &'static str> {
impl BuiltinScalarFunction {
/// an allowlist of functions to take zero arguments, so that they will get special treatment
/// while executing.
#[deprecated(
since = "32.0.0",
note = "please use TypeSignature::supports_zero_argument instead"
)]
pub fn supports_zero_argument(&self) -> bool {
matches!(
self,
BuiltinScalarFunction::Pi
| BuiltinScalarFunction::Random
| BuiltinScalarFunction::Now
| BuiltinScalarFunction::CurrentDate
| BuiltinScalarFunction::CurrentTime
| BuiltinScalarFunction::Uuid
| BuiltinScalarFunction::MakeArray
)
self.signature().type_signature.supports_zero_argument()
}

/// Returns the [Volatility] of the builtin function.
pub fn volatility(&self) -> Volatility {
match self {
Expand Down Expand Up @@ -494,7 +490,9 @@ impl BuiltinScalarFunction {
// Note that this function *must* return the same type that the respective physical expression returns
// or the execution panics.

if input_expr_types.is_empty() && !self.supports_zero_argument() {
if input_expr_types.is_empty()
&& !self.signature().type_signature.supports_zero_argument()
{
return plan_err!(
"{}",
utils::generate_signature_error_msg(
Expand Down Expand Up @@ -904,7 +902,8 @@ impl BuiltinScalarFunction {
}
BuiltinScalarFunction::Cardinality => Signature::any(1, self.volatility()),
BuiltinScalarFunction::MakeArray => {
Signature::variadic_any(self.volatility())
// 0 or more arguments of arbitrary type
Signature::one_of(vec![VariadicAny, Any(0)], self.volatility())
}
BuiltinScalarFunction::Struct => Signature::variadic(
struct_expressions::SUPPORTED_STRUCT_TYPES.to_vec(),
Expand Down
75 changes: 70 additions & 5 deletions datafusion/expr/src/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,31 +82,36 @@ pub enum Volatility {
/// ```
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum TypeSignature {
/// arbitrary number of arguments of an common type out of a list of valid types.
/// One or more arguments of an common type out of a list of valid types.
///
/// # Examples
/// A function such as `concat` is `Variadic(vec![DataType::Utf8, DataType::LargeUtf8])`
Variadic(Vec<DataType>),
/// arbitrary number of arguments of an arbitrary but equal type.
/// One or more arguments of an arbitrary but equal type.
/// DataFusion attempts to coerce all argument types to match the first argument's type
///
/// # Examples
/// A function such as `array` is `VariadicEqual`
VariadicEqual,
/// arbitrary number of arguments with arbitrary types
/// One or more arguments with arbitrary types
VariadicAny,
/// fixed number of arguments of an arbitrary but equal type out of a list of valid types.
///
/// # Examples
/// 1. A function of one argument of f64 is `Uniform(1, vec![DataType::Float64])`
/// 2. A function of one argument of f64 or f32 is `Uniform(1, vec![DataType::Float32, DataType::Float64])`
Uniform(usize, Vec<DataType>),
/// exact number of arguments of an exact type
/// Exact number of arguments of an exact type
Exact(Vec<DataType>),
/// fixed number of arguments of arbitrary types
/// Fixed number of arguments of arbitrary types
/// If a function takes 0 argument, its `TypeSignature` should be `Any(0)`
Any(usize),
/// Matches exactly one of a list of [`TypeSignature`]s. Coercion is attempted to match
/// the signatures in order, and stops after the first success, if any.
///
/// # Examples
/// Function `make_array` takes 0 or more arguments with arbitrary types, its `TypeSignature`
/// is `OneOf(vec![Any(0), VariadicAny])`.
OneOf(Vec<TypeSignature>),
}

Expand Down Expand Up @@ -150,6 +155,18 @@ impl TypeSignature {
.collect::<Vec<String>>()
.join(delimiter)
}

/// Check whether 0 input argument is valid for given `TypeSignature`
pub fn supports_zero_argument(&self) -> bool {
match &self {
TypeSignature::Exact(vec) => vec.is_empty(),
TypeSignature::Uniform(0, _) | TypeSignature::Any(0) => true,
TypeSignature::OneOf(types) => types
.iter()
.any(|type_sig| type_sig.supports_zero_argument()),
_ => false,
}
}
}

/// Defines the supported argument types ([`TypeSignature`]) and [`Volatility`] for a function.
Expand Down Expand Up @@ -234,3 +251,51 @@ impl Signature {
/// - `Some(true)` indicates that the function is monotonically increasing w.r.t. the argument in question.
/// - Some(false) indicates that the function is monotonically decreasing w.r.t. the argument in question.
pub type FuncMonotonicity = Vec<Option<bool>>;

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn supports_zero_argument_tests() {
// Testing `TypeSignature`s which supports 0 arg
let positive_cases = vec![
TypeSignature::Exact(vec![]),
TypeSignature::Uniform(0, vec![DataType::Float64]),
TypeSignature::Any(0),
TypeSignature::OneOf(vec![
TypeSignature::Exact(vec![DataType::Int8]),
TypeSignature::Any(0),
TypeSignature::Uniform(1, vec![DataType::Int8]),
]),
];

for case in positive_cases {
assert!(
case.supports_zero_argument(),
"Expected {:?} to support zero arguments",
case
);
}

// Testing `TypeSignature`s which doesn't support 0 arg
let negative_cases = vec![
TypeSignature::Exact(vec![DataType::Utf8]),
TypeSignature::Uniform(1, vec![DataType::Float64]),
TypeSignature::Any(1),
TypeSignature::VariadicAny,
TypeSignature::OneOf(vec![
TypeSignature::Exact(vec![DataType::Int8]),
TypeSignature::Uniform(1, vec![DataType::Int8]),
]),
];

for case in negative_cases {
assert!(
!case.supports_zero_argument(),
"Expected {:?} not to support zero arguments",
case
);
}
}
}
5 changes: 4 additions & 1 deletion datafusion/physical-expr/src/scalar_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,10 @@ impl PhysicalExpr for ScalarFunctionExpr {
let inputs = match (self.args.len(), self.name.parse::<BuiltinScalarFunction>()) {
// MakeArray support zero argument but has the different behavior from the array with one null.
(0, Ok(scalar_fun))
if scalar_fun.supports_zero_argument()
if scalar_fun
.signature()
.type_signature
.supports_zero_argument()
&& scalar_fun != BuiltinScalarFunction::MakeArray =>
{
vec![ColumnarValue::create_null_array(batch.num_rows())]
Expand Down

0 comments on commit 308c354

Please sign in to comment.