Skip to content

Commit 308c354

Browse files
authored
Minor: Remove redundant BuiltinScalarFunction::supports_zero_argument() (#8059)
* deprecate BuiltinScalarFunction::supports_zero_argument() * unify old supports_zero_argument() impl
1 parent af3ce6b commit 308c354

File tree

3 files changed

+85
-18
lines changed

3 files changed

+85
-18
lines changed

datafusion/expr/src/built_in_function.rs

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -325,18 +325,14 @@ fn function_to_name() -> &'static HashMap<BuiltinScalarFunction, &'static str> {
325325
impl BuiltinScalarFunction {
326326
/// an allowlist of functions to take zero arguments, so that they will get special treatment
327327
/// while executing.
328+
#[deprecated(
329+
since = "32.0.0",
330+
note = "please use TypeSignature::supports_zero_argument instead"
331+
)]
328332
pub fn supports_zero_argument(&self) -> bool {
329-
matches!(
330-
self,
331-
BuiltinScalarFunction::Pi
332-
| BuiltinScalarFunction::Random
333-
| BuiltinScalarFunction::Now
334-
| BuiltinScalarFunction::CurrentDate
335-
| BuiltinScalarFunction::CurrentTime
336-
| BuiltinScalarFunction::Uuid
337-
| BuiltinScalarFunction::MakeArray
338-
)
333+
self.signature().type_signature.supports_zero_argument()
339334
}
335+
340336
/// Returns the [Volatility] of the builtin function.
341337
pub fn volatility(&self) -> Volatility {
342338
match self {
@@ -494,7 +490,9 @@ impl BuiltinScalarFunction {
494490
// Note that this function *must* return the same type that the respective physical expression returns
495491
// or the execution panics.
496492

497-
if input_expr_types.is_empty() && !self.supports_zero_argument() {
493+
if input_expr_types.is_empty()
494+
&& !self.signature().type_signature.supports_zero_argument()
495+
{
498496
return plan_err!(
499497
"{}",
500498
utils::generate_signature_error_msg(
@@ -904,7 +902,8 @@ impl BuiltinScalarFunction {
904902
}
905903
BuiltinScalarFunction::Cardinality => Signature::any(1, self.volatility()),
906904
BuiltinScalarFunction::MakeArray => {
907-
Signature::variadic_any(self.volatility())
905+
// 0 or more arguments of arbitrary type
906+
Signature::one_of(vec![VariadicAny, Any(0)], self.volatility())
908907
}
909908
BuiltinScalarFunction::Struct => Signature::variadic(
910909
struct_expressions::SUPPORTED_STRUCT_TYPES.to_vec(),

datafusion/expr/src/signature.rs

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,31 +82,36 @@ pub enum Volatility {
8282
/// ```
8383
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
8484
pub enum TypeSignature {
85-
/// arbitrary number of arguments of an common type out of a list of valid types.
85+
/// One or more arguments of an common type out of a list of valid types.
8686
///
8787
/// # Examples
8888
/// A function such as `concat` is `Variadic(vec![DataType::Utf8, DataType::LargeUtf8])`
8989
Variadic(Vec<DataType>),
90-
/// arbitrary number of arguments of an arbitrary but equal type.
90+
/// One or more arguments of an arbitrary but equal type.
9191
/// DataFusion attempts to coerce all argument types to match the first argument's type
9292
///
9393
/// # Examples
9494
/// A function such as `array` is `VariadicEqual`
9595
VariadicEqual,
96-
/// arbitrary number of arguments with arbitrary types
96+
/// One or more arguments with arbitrary types
9797
VariadicAny,
9898
/// fixed number of arguments of an arbitrary but equal type out of a list of valid types.
9999
///
100100
/// # Examples
101101
/// 1. A function of one argument of f64 is `Uniform(1, vec![DataType::Float64])`
102102
/// 2. A function of one argument of f64 or f32 is `Uniform(1, vec![DataType::Float32, DataType::Float64])`
103103
Uniform(usize, Vec<DataType>),
104-
/// exact number of arguments of an exact type
104+
/// Exact number of arguments of an exact type
105105
Exact(Vec<DataType>),
106-
/// fixed number of arguments of arbitrary types
106+
/// Fixed number of arguments of arbitrary types
107+
/// If a function takes 0 argument, its `TypeSignature` should be `Any(0)`
107108
Any(usize),
108109
/// Matches exactly one of a list of [`TypeSignature`]s. Coercion is attempted to match
109110
/// the signatures in order, and stops after the first success, if any.
111+
///
112+
/// # Examples
113+
/// Function `make_array` takes 0 or more arguments with arbitrary types, its `TypeSignature`
114+
/// is `OneOf(vec![Any(0), VariadicAny])`.
110115
OneOf(Vec<TypeSignature>),
111116
}
112117

@@ -150,6 +155,18 @@ impl TypeSignature {
150155
.collect::<Vec<String>>()
151156
.join(delimiter)
152157
}
158+
159+
/// Check whether 0 input argument is valid for given `TypeSignature`
160+
pub fn supports_zero_argument(&self) -> bool {
161+
match &self {
162+
TypeSignature::Exact(vec) => vec.is_empty(),
163+
TypeSignature::Uniform(0, _) | TypeSignature::Any(0) => true,
164+
TypeSignature::OneOf(types) => types
165+
.iter()
166+
.any(|type_sig| type_sig.supports_zero_argument()),
167+
_ => false,
168+
}
169+
}
153170
}
154171

155172
/// Defines the supported argument types ([`TypeSignature`]) and [`Volatility`] for a function.
@@ -234,3 +251,51 @@ impl Signature {
234251
/// - `Some(true)` indicates that the function is monotonically increasing w.r.t. the argument in question.
235252
/// - Some(false) indicates that the function is monotonically decreasing w.r.t. the argument in question.
236253
pub type FuncMonotonicity = Vec<Option<bool>>;
254+
255+
#[cfg(test)]
256+
mod tests {
257+
use super::*;
258+
259+
#[test]
260+
fn supports_zero_argument_tests() {
261+
// Testing `TypeSignature`s which supports 0 arg
262+
let positive_cases = vec![
263+
TypeSignature::Exact(vec![]),
264+
TypeSignature::Uniform(0, vec![DataType::Float64]),
265+
TypeSignature::Any(0),
266+
TypeSignature::OneOf(vec![
267+
TypeSignature::Exact(vec![DataType::Int8]),
268+
TypeSignature::Any(0),
269+
TypeSignature::Uniform(1, vec![DataType::Int8]),
270+
]),
271+
];
272+
273+
for case in positive_cases {
274+
assert!(
275+
case.supports_zero_argument(),
276+
"Expected {:?} to support zero arguments",
277+
case
278+
);
279+
}
280+
281+
// Testing `TypeSignature`s which doesn't support 0 arg
282+
let negative_cases = vec![
283+
TypeSignature::Exact(vec![DataType::Utf8]),
284+
TypeSignature::Uniform(1, vec![DataType::Float64]),
285+
TypeSignature::Any(1),
286+
TypeSignature::VariadicAny,
287+
TypeSignature::OneOf(vec![
288+
TypeSignature::Exact(vec![DataType::Int8]),
289+
TypeSignature::Uniform(1, vec![DataType::Int8]),
290+
]),
291+
];
292+
293+
for case in negative_cases {
294+
assert!(
295+
!case.supports_zero_argument(),
296+
"Expected {:?} not to support zero arguments",
297+
case
298+
);
299+
}
300+
}
301+
}

datafusion/physical-expr/src/scalar_function.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,10 @@ impl PhysicalExpr for ScalarFunctionExpr {
136136
let inputs = match (self.args.len(), self.name.parse::<BuiltinScalarFunction>()) {
137137
// MakeArray support zero argument but has the different behavior from the array with one null.
138138
(0, Ok(scalar_fun))
139-
if scalar_fun.supports_zero_argument()
139+
if scalar_fun
140+
.signature()
141+
.type_signature
142+
.supports_zero_argument()
140143
&& scalar_fun != BuiltinScalarFunction::MakeArray =>
141144
{
142145
vec![ColumnarValue::create_null_array(batch.num_rows())]

0 commit comments

Comments
 (0)