diff --git a/datafusion/functions-aggregate/src/first_last.rs b/datafusion/functions-aggregate/src/first_last.rs index fd4e21971028..6fcb655ae432 100644 --- a/datafusion/functions-aggregate/src/first_last.rs +++ b/datafusion/functions-aggregate/src/first_last.rs @@ -72,7 +72,7 @@ impl Default for FirstValue { impl FirstValue { pub fn new() -> Self { Self { - aliases: vec![String::from("FIRST_VALUE"), String::from("first_value")], + aliases: vec![String::from("first_value")], signature: Signature::one_of( vec![ // TODO: we can introduce more strict signature that only numeric of array types are allowed @@ -372,7 +372,7 @@ impl Default for LastValue { impl LastValue { pub fn new() -> Self { Self { - aliases: vec![String::from("LAST_VALUE"), String::from("last_value")], + aliases: vec![String::from("last_value")], signature: Signature::one_of( vec![ // TODO: we can introduce more strict signature that only numeric of array types are allowed diff --git a/datafusion/functions-aggregate/src/lib.rs b/datafusion/functions-aggregate/src/lib.rs index ac40a90aaec6..0c13952cc57f 100644 --- a/datafusion/functions-aggregate/src/lib.rs +++ b/datafusion/functions-aggregate/src/lib.rs @@ -72,15 +72,20 @@ pub mod expr_fn { pub use super::median::median; } -/// Registers all enabled packages with a [`FunctionRegistry`] -pub fn register_all(registry: &mut dyn FunctionRegistry) -> Result<()> { - let functions: Vec> = vec![ +/// Returns all default aggregate functions +pub fn all_default_aggregate_functions() -> Vec> { + vec![ first_last::first_value_udaf(), first_last::last_value_udaf(), covariance::covar_samp_udaf(), covariance::covar_pop_udaf(), median::median_udaf(), - ]; + ] +} + +/// Registers all enabled packages with a [`FunctionRegistry`] +pub fn register_all(registry: &mut dyn FunctionRegistry) -> Result<()> { + let functions: Vec> = all_default_aggregate_functions(); functions.into_iter().try_for_each(|udf| { let existing_udaf = registry.register_udaf(udf)?; @@ -92,3 +97,30 @@ pub fn register_all(registry: &mut dyn FunctionRegistry) -> Result<()> { Ok(()) } + +#[cfg(test)] +mod tests { + use crate::all_default_aggregate_functions; + use datafusion_common::Result; + use std::collections::HashSet; + + #[test] + fn test_no_duplicate_name() -> Result<()> { + let mut names = HashSet::new(); + for func in all_default_aggregate_functions() { + assert!( + names.insert(func.name().to_string()), + "duplicate function name: {}", + func.name() + ); + for alias in func.aliases() { + assert!( + names.insert(alias.to_string()), + "duplicate function name: {}", + alias + ); + } + } + Ok(()) + } +} diff --git a/datafusion/functions-array/src/array_has.rs b/datafusion/functions-array/src/array_has.rs index 466a913e35e5..136c6e769120 100644 --- a/datafusion/functions-array/src/array_has.rs +++ b/datafusion/functions-array/src/array_has.rs @@ -68,7 +68,6 @@ impl ArrayHas { Self { signature: Signature::array_and_element(Volatility::Immutable), aliases: vec![ - String::from("array_has"), String::from("list_has"), String::from("array_contains"), String::from("list_contains"), @@ -140,7 +139,7 @@ impl ArrayHasAll { pub fn new() -> Self { Self { signature: Signature::any(2, Volatility::Immutable), - aliases: vec![String::from("array_has_all"), String::from("list_has_all")], + aliases: vec![String::from("list_has_all")], } } } @@ -203,7 +202,7 @@ impl ArrayHasAny { pub fn new() -> Self { Self { signature: Signature::any(2, Volatility::Immutable), - aliases: vec![String::from("array_has_any"), String::from("list_has_any")], + aliases: vec![String::from("list_has_any")], } } } diff --git a/datafusion/functions-array/src/cardinality.rs b/datafusion/functions-array/src/cardinality.rs index d17965b795ad..f6755c344768 100644 --- a/datafusion/functions-array/src/cardinality.rs +++ b/datafusion/functions-array/src/cardinality.rs @@ -40,7 +40,7 @@ impl Cardinality { pub fn new() -> Self { Self { signature: Signature::array(Volatility::Immutable), - aliases: vec![String::from("cardinality")], + aliases: vec![], } } } diff --git a/datafusion/functions-array/src/concat.rs b/datafusion/functions-array/src/concat.rs index d49cef66742f..330c50f5b055 100644 --- a/datafusion/functions-array/src/concat.rs +++ b/datafusion/functions-array/src/concat.rs @@ -59,7 +59,6 @@ impl ArrayAppend { Self { signature: Signature::array_and_element(Volatility::Immutable), aliases: vec![ - String::from("array_append"), String::from("list_append"), String::from("array_push_back"), String::from("list_push_back"), @@ -119,7 +118,6 @@ impl ArrayPrepend { Self { signature: Signature::element_and_array(Volatility::Immutable), aliases: vec![ - String::from("array_prepend"), String::from("list_prepend"), String::from("array_push_front"), String::from("list_push_front"), @@ -178,7 +176,6 @@ impl ArrayConcat { Self { signature: Signature::variadic_any(Volatility::Immutable), aliases: vec![ - String::from("array_concat"), String::from("array_cat"), String::from("list_concat"), String::from("list_cat"), diff --git a/datafusion/functions-array/src/dimension.rs b/datafusion/functions-array/src/dimension.rs index 0c65da283bbb..d84fa0c19ee9 100644 --- a/datafusion/functions-array/src/dimension.rs +++ b/datafusion/functions-array/src/dimension.rs @@ -50,7 +50,7 @@ impl ArrayDims { pub fn new() -> Self { Self { signature: Signature::array(Volatility::Immutable), - aliases: vec!["array_dims".to_string(), "list_dims".to_string()], + aliases: vec!["list_dims".to_string()], } } } @@ -104,7 +104,7 @@ impl ArrayNdims { pub fn new() -> Self { Self { signature: Signature::array(Volatility::Immutable), - aliases: vec![String::from("array_ndims"), String::from("list_ndims")], + aliases: vec![String::from("list_ndims")], } } } diff --git a/datafusion/functions-array/src/empty.rs b/datafusion/functions-array/src/empty.rs index c5fe74480fb5..48435b7fa730 100644 --- a/datafusion/functions-array/src/empty.rs +++ b/datafusion/functions-array/src/empty.rs @@ -44,11 +44,7 @@ impl ArrayEmpty { pub fn new() -> Self { Self { signature: Signature::array(Volatility::Immutable), - aliases: vec![ - "empty".to_string(), - "array_empty".to_string(), - "list_empty".to_string(), - ], + aliases: vec!["array_empty".to_string(), "list_empty".to_string()], } } } diff --git a/datafusion/functions-array/src/except.rs b/datafusion/functions-array/src/except.rs index 453b4f77119d..50ef20a7d416 100644 --- a/datafusion/functions-array/src/except.rs +++ b/datafusion/functions-array/src/except.rs @@ -47,7 +47,7 @@ impl ArrayExcept { pub fn new() -> Self { Self { signature: Signature::any(2, Volatility::Immutable), - aliases: vec!["array_except".to_string(), "list_except".to_string()], + aliases: vec!["list_except".to_string()], } } } diff --git a/datafusion/functions-array/src/extract.rs b/datafusion/functions-array/src/extract.rs index a12cdc20dfa3..a2ee6f2ef5f9 100644 --- a/datafusion/functions-array/src/extract.rs +++ b/datafusion/functions-array/src/extract.rs @@ -80,7 +80,6 @@ impl ArrayElement { Self { signature: Signature::array_and_index(Volatility::Immutable), aliases: vec![ - String::from("array_element"), String::from("array_extract"), String::from("list_element"), String::from("list_extract"), @@ -241,7 +240,7 @@ impl ArraySlice { pub fn new() -> Self { Self { signature: Signature::variadic_any(Volatility::Immutable), - aliases: vec![String::from("array_slice"), String::from("list_slice")], + aliases: vec![String::from("list_slice")], } } } @@ -513,10 +512,7 @@ impl ArrayPopFront { pub fn new() -> Self { Self { signature: Signature::array(Volatility::Immutable), - aliases: vec![ - String::from("array_pop_front"), - String::from("list_pop_front"), - ], + aliases: vec![String::from("list_pop_front")], } } } @@ -591,10 +587,7 @@ impl ArrayPopBack { pub fn new() -> Self { Self { signature: Signature::array(Volatility::Immutable), - aliases: vec![ - String::from("array_pop_back"), - String::from("list_pop_back"), - ], + aliases: vec![String::from("list_pop_back")], } } } diff --git a/datafusion/functions-array/src/flatten.rs b/datafusion/functions-array/src/flatten.rs index 41762157fc6a..a495c3ade96f 100644 --- a/datafusion/functions-array/src/flatten.rs +++ b/datafusion/functions-array/src/flatten.rs @@ -47,7 +47,7 @@ impl Flatten { pub fn new() -> Self { Self { signature: Signature::array(Volatility::Immutable), - aliases: vec![String::from("flatten")], + aliases: vec![], } } } diff --git a/datafusion/functions-array/src/length.rs b/datafusion/functions-array/src/length.rs index ed04c52584c0..5d9ccd2901cf 100644 --- a/datafusion/functions-array/src/length.rs +++ b/datafusion/functions-array/src/length.rs @@ -48,7 +48,7 @@ impl ArrayLength { pub fn new() -> Self { Self { signature: Signature::variadic_any(Volatility::Immutable), - aliases: vec![String::from("array_length"), String::from("list_length")], + aliases: vec![String::from("list_length")], } } } diff --git a/datafusion/functions-array/src/lib.rs b/datafusion/functions-array/src/lib.rs index 93be8bd79063..63ba5d14834d 100644 --- a/datafusion/functions-array/src/lib.rs +++ b/datafusion/functions-array/src/lib.rs @@ -99,9 +99,9 @@ pub mod expr_fn { pub use super::string::string_to_array; } -/// Registers all enabled packages with a [`FunctionRegistry`] -pub fn register_all(registry: &mut dyn FunctionRegistry) -> Result<()> { - let functions: Vec> = vec![ +/// Return all default array functions +pub fn all_default_array_functions() -> Vec> { + vec![ string::array_to_string_udf(), string::string_to_array_udf(), range::range_udf(), @@ -139,7 +139,12 @@ pub fn register_all(registry: &mut dyn FunctionRegistry) -> Result<()> { replace::array_replace_n_udf(), replace::array_replace_all_udf(), replace::array_replace_udf(), - ]; + ] +} + +/// Registers all enabled packages with a [`FunctionRegistry`] +pub fn register_all(registry: &mut dyn FunctionRegistry) -> Result<()> { + let functions: Vec> = all_default_array_functions(); functions.into_iter().try_for_each(|udf| { let existing_udf = registry.register_udf(udf)?; if let Some(existing_udf) = existing_udf { @@ -151,3 +156,30 @@ pub fn register_all(registry: &mut dyn FunctionRegistry) -> Result<()> { Ok(()) } + +#[cfg(test)] +mod tests { + use crate::all_default_array_functions; + use datafusion_common::Result; + use std::collections::HashSet; + + #[test] + fn test_no_duplicate_name() -> Result<()> { + let mut names = HashSet::new(); + for func in all_default_array_functions() { + assert!( + names.insert(func.name().to_string()), + "duplicate function name: {}", + func.name() + ); + for alias in func.aliases() { + assert!( + names.insert(alias.to_string()), + "duplicate function name: {}", + alias + ); + } + } + Ok(()) + } +} diff --git a/datafusion/functions-array/src/position.rs b/datafusion/functions-array/src/position.rs index 0002d5c40b3e..a48332ceb0b3 100644 --- a/datafusion/functions-array/src/position.rs +++ b/datafusion/functions-array/src/position.rs @@ -55,7 +55,6 @@ impl ArrayPosition { Volatility::Immutable, ), aliases: vec![ - String::from("array_position"), String::from("list_position"), String::from("array_indexof"), String::from("list_indexof"), @@ -183,10 +182,7 @@ impl ArrayPositions { pub fn new() -> Self { Self { signature: Signature::array_and_element(Volatility::Immutable), - aliases: vec![ - String::from("array_positions"), - String::from("list_positions"), - ], + aliases: vec![String::from("list_positions")], } } } diff --git a/datafusion/functions-array/src/range.rs b/datafusion/functions-array/src/range.rs index 881e86d63ab8..8c73bd821346 100644 --- a/datafusion/functions-array/src/range.rs +++ b/datafusion/functions-array/src/range.rs @@ -58,7 +58,7 @@ impl Range { ], Volatility::Immutable, ), - aliases: vec![String::from("range")], + aliases: vec![], } } } @@ -129,7 +129,7 @@ impl GenSeries { ], Volatility::Immutable, ), - aliases: vec![String::from("generate_series")], + aliases: vec![], } } } diff --git a/datafusion/functions-array/src/remove.rs b/datafusion/functions-array/src/remove.rs index 8c408f1650e9..589dd4d0c41c 100644 --- a/datafusion/functions-array/src/remove.rs +++ b/datafusion/functions-array/src/remove.rs @@ -49,7 +49,7 @@ impl ArrayRemove { pub fn new() -> Self { Self { signature: Signature::array_and_element(Volatility::Immutable), - aliases: vec!["array_remove".to_string(), "list_remove".to_string()], + aliases: vec!["list_remove".to_string()], } } } @@ -98,7 +98,7 @@ impl ArrayRemoveN { pub fn new() -> Self { Self { signature: Signature::any(3, Volatility::Immutable), - aliases: vec!["array_remove_n".to_string(), "list_remove_n".to_string()], + aliases: vec!["list_remove_n".to_string()], } } } @@ -147,10 +147,7 @@ impl ArrayRemoveAll { pub fn new() -> Self { Self { signature: Signature::array_and_element(Volatility::Immutable), - aliases: vec![ - "array_remove_all".to_string(), - "list_remove_all".to_string(), - ], + aliases: vec!["list_remove_all".to_string()], } } } diff --git a/datafusion/functions-array/src/repeat.rs b/datafusion/functions-array/src/repeat.rs index 78bcde9eaba7..7ed913da3f2a 100644 --- a/datafusion/functions-array/src/repeat.rs +++ b/datafusion/functions-array/src/repeat.rs @@ -50,7 +50,7 @@ impl ArrayRepeat { pub fn new() -> Self { Self { signature: Signature::variadic_any(Volatility::Immutable), - aliases: vec![String::from("array_repeat"), String::from("list_repeat")], + aliases: vec![String::from("list_repeat")], } } } diff --git a/datafusion/functions-array/src/replace.rs b/datafusion/functions-array/src/replace.rs index 8ac32538ad4f..46a2e078aa4c 100644 --- a/datafusion/functions-array/src/replace.rs +++ b/datafusion/functions-array/src/replace.rs @@ -65,7 +65,7 @@ impl ArrayReplace { pub fn new() -> Self { Self { signature: Signature::any(3, Volatility::Immutable), - aliases: vec![String::from("array_replace"), String::from("list_replace")], + aliases: vec![String::from("list_replace")], } } } @@ -106,10 +106,7 @@ impl ArrayReplaceN { pub fn new() -> Self { Self { signature: Signature::any(4, Volatility::Immutable), - aliases: vec![ - String::from("array_replace_n"), - String::from("list_replace_n"), - ], + aliases: vec![String::from("list_replace_n")], } } } @@ -150,10 +147,7 @@ impl ArrayReplaceAll { pub fn new() -> Self { Self { signature: Signature::any(3, Volatility::Immutable), - aliases: vec![ - String::from("array_replace_all"), - String::from("list_replace_all"), - ], + aliases: vec![String::from("list_replace_all")], } } } diff --git a/datafusion/functions-array/src/resize.rs b/datafusion/functions-array/src/resize.rs index 7028bd1c33cc..078ec7766aac 100644 --- a/datafusion/functions-array/src/resize.rs +++ b/datafusion/functions-array/src/resize.rs @@ -47,7 +47,7 @@ impl ArrayResize { pub fn new() -> Self { Self { signature: Signature::variadic_any(Volatility::Immutable), - aliases: vec!["array_resize".to_string(), "list_resize".to_string()], + aliases: vec!["list_resize".to_string()], } } } diff --git a/datafusion/functions-array/src/reverse.rs b/datafusion/functions-array/src/reverse.rs index c9988524cabd..b462be40209b 100644 --- a/datafusion/functions-array/src/reverse.rs +++ b/datafusion/functions-array/src/reverse.rs @@ -47,7 +47,7 @@ impl ArrayReverse { pub fn new() -> Self { Self { signature: Signature::any(1, Volatility::Immutable), - aliases: vec!["array_reverse".to_string(), "list_reverse".to_string()], + aliases: vec!["list_reverse".to_string()], } } } diff --git a/datafusion/functions-array/src/set_ops.rs b/datafusion/functions-array/src/set_ops.rs index 9032a745ef7a..2facc51d508f 100644 --- a/datafusion/functions-array/src/set_ops.rs +++ b/datafusion/functions-array/src/set_ops.rs @@ -69,7 +69,7 @@ impl ArrayUnion { pub fn new() -> Self { Self { signature: Signature::any(2, Volatility::Immutable), - aliases: vec![String::from("array_union"), String::from("list_union")], + aliases: vec![String::from("list_union")], } } } @@ -114,10 +114,7 @@ impl ArrayIntersect { pub fn new() -> Self { Self { signature: Signature::any(2, Volatility::Immutable), - aliases: vec![ - String::from("array_intersect"), - String::from("list_intersect"), - ], + aliases: vec![String::from("list_intersect")], } } } @@ -162,7 +159,7 @@ impl ArrayDistinct { pub fn new() -> Self { Self { signature: Signature::array(Volatility::Immutable), - aliases: vec!["array_distinct".to_string(), "list_distinct".to_string()], + aliases: vec!["list_distinct".to_string()], } } } diff --git a/datafusion/functions-array/src/sort.rs b/datafusion/functions-array/src/sort.rs index 2a554bf3d9da..0230c42175a5 100644 --- a/datafusion/functions-array/src/sort.rs +++ b/datafusion/functions-array/src/sort.rs @@ -47,7 +47,7 @@ impl ArraySort { pub fn new() -> Self { Self { signature: Signature::variadic_any(Volatility::Immutable), - aliases: vec!["array_sort".to_string(), "list_sort".to_string()], + aliases: vec!["list_sort".to_string()], } } } diff --git a/datafusion/functions-array/src/string.rs b/datafusion/functions-array/src/string.rs index e14e315752b4..04832b4b1259 100644 --- a/datafusion/functions-array/src/string.rs +++ b/datafusion/functions-array/src/string.rs @@ -119,7 +119,6 @@ impl ArrayToString { Self { signature: Signature::variadic_any(Volatility::Immutable), aliases: vec![ - String::from("array_to_string"), String::from("list_to_string"), String::from("array_join"), String::from("list_join"), @@ -182,10 +181,7 @@ impl StringToArray { ], Volatility::Immutable, ), - aliases: vec![ - String::from("string_to_array"), - String::from("string_to_list"), - ], + aliases: vec![String::from("string_to_list")], } } } diff --git a/datafusion/functions/src/lib.rs b/datafusion/functions/src/lib.rs index 2a00839dc532..e36c491fefbb 100644 --- a/datafusion/functions/src/lib.rs +++ b/datafusion/functions/src/lib.rs @@ -79,7 +79,9 @@ //! [`ScalarUDF`]: datafusion_expr::ScalarUDF use datafusion_common::Result; use datafusion_execution::FunctionRegistry; +use datafusion_expr::ScalarUDF; use log::debug; +use std::sync::Arc; #[macro_use] pub mod macros; @@ -150,9 +152,9 @@ pub mod expr_fn { pub use super::unicode::expr_fn::*; } -/// Registers all enabled packages with a [`FunctionRegistry`] -pub fn register_all(registry: &mut dyn FunctionRegistry) -> Result<()> { - let mut all_functions = core::functions() +/// Return all default functions +pub fn all_default_functions() -> Vec> { + core::functions() .into_iter() .chain(datetime::functions()) .chain(encoding::functions()) @@ -160,9 +162,15 @@ pub fn register_all(registry: &mut dyn FunctionRegistry) -> Result<()> { .chain(regex::functions()) .chain(crypto::functions()) .chain(unicode::functions()) - .chain(string::functions()); + .chain(string::functions()) + .collect::>() +} + +/// Registers all enabled packages with a [`FunctionRegistry`] +pub fn register_all(registry: &mut dyn FunctionRegistry) -> Result<()> { + let all_functions = all_default_functions(); - all_functions.try_for_each(|udf| { + all_functions.into_iter().try_for_each(|udf| { let existing_udf = registry.register_udf(udf)?; if let Some(existing_udf) = existing_udf { debug!("Overwrite existing UDF: {}", existing_udf.name()); @@ -171,3 +179,30 @@ pub fn register_all(registry: &mut dyn FunctionRegistry) -> Result<()> { })?; Ok(()) } + +#[cfg(test)] +mod tests { + use crate::all_default_functions; + use datafusion_common::Result; + use std::collections::HashSet; + + #[test] + fn test_no_duplicate_name() -> Result<()> { + let mut names = HashSet::new(); + for func in all_default_functions() { + assert!( + names.insert(func.name().to_string()), + "duplicate function name: {}", + func.name() + ); + for alias in func.aliases() { + assert!( + names.insert(alias.to_string()), + "duplicate function name: {}", + alias + ); + } + } + Ok(()) + } +} diff --git a/datafusion/optimizer/src/push_down_limit.rs b/datafusion/optimizer/src/push_down_limit.rs index b97dff74d979..6723672ff498 100644 --- a/datafusion/optimizer/src/push_down_limit.rs +++ b/datafusion/optimizer/src/push_down_limit.rs @@ -65,7 +65,6 @@ impl OptimizerRule for PushDownLimit { }; let Limit { skip, fetch, input } = limit; - let input = input; // Merge the Parent Limit and the Child Limit. if let LogicalPlan::Limit(child) = input.as_ref() {