Skip to content

Remove duplicate function name in its aliases list #10661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions datafusion/functions-aggregate/src/first_last.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
40 changes: 36 additions & 4 deletions datafusion/functions-aggregate/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Arc<AggregateUDF>> = vec![
/// Returns all default aggregate functions
pub fn all_default_aggregate_functions() -> Vec<Arc<AggregateUDF>> {
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<Arc<AggregateUDF>> = all_default_aggregate_functions();

functions.into_iter().try_for_each(|udf| {
let existing_udaf = registry.register_udaf(udf)?;
Expand All @@ -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(())
}
}
5 changes: 2 additions & 3 deletions datafusion/functions-array/src/array_has.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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")],
}
}
}
Expand Down Expand Up @@ -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")],
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion datafusion/functions-array/src/cardinality.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl Cardinality {
pub fn new() -> Self {
Self {
signature: Signature::array(Volatility::Immutable),
aliases: vec![String::from("cardinality")],
aliases: vec![],
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions datafusion/functions-array/src/concat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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"),
Expand Down
4 changes: 2 additions & 2 deletions datafusion/functions-array/src/dimension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()],
}
}
}
Expand Down Expand Up @@ -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")],
}
}
}
Expand Down
6 changes: 1 addition & 5 deletions datafusion/functions-array/src/empty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()],
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion datafusion/functions-array/src/except.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()],
}
}
}
Expand Down
13 changes: 3 additions & 10 deletions datafusion/functions-array/src/extract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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")],
}
}
}
Expand Down Expand Up @@ -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")],
}
}
}
Expand Down Expand Up @@ -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")],
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion datafusion/functions-array/src/flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl Flatten {
pub fn new() -> Self {
Self {
signature: Signature::array(Volatility::Immutable),
aliases: vec![String::from("flatten")],
aliases: vec![],
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion datafusion/functions-array/src/length.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")],
}
}
}
Expand Down
40 changes: 36 additions & 4 deletions datafusion/functions-array/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Arc<ScalarUDF>> = vec![
/// Return all default array functions
pub fn all_default_array_functions() -> Vec<Arc<ScalarUDF>> {
vec![
string::array_to_string_udf(),
string::string_to_array_udf(),
range::range_udf(),
Expand Down Expand Up @@ -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<Arc<ScalarUDF>> = 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 {
Expand All @@ -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(())
}
}
6 changes: 1 addition & 5 deletions datafusion/functions-array/src/position.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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")],
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions datafusion/functions-array/src/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl Range {
],
Volatility::Immutable,
),
aliases: vec![String::from("range")],
aliases: vec![],
}
}
}
Expand Down Expand Up @@ -129,7 +129,7 @@ impl GenSeries {
],
Volatility::Immutable,
),
aliases: vec![String::from("generate_series")],
aliases: vec![],
}
}
}
Expand Down
9 changes: 3 additions & 6 deletions datafusion/functions-array/src/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()],
}
}
}
Expand Down Expand Up @@ -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()],
}
}
}
Expand Down Expand Up @@ -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()],
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion datafusion/functions-array/src/repeat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")],
}
}
}
Expand Down
12 changes: 3 additions & 9 deletions datafusion/functions-array/src/replace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")],
}
}
}
Expand Down Expand Up @@ -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")],
}
}
}
Expand Down Expand Up @@ -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")],
}
}
}
Expand Down
Loading