Skip to content

Commit

Permalink
fix: DataFusion suggests invalid functions (#8083)
Browse files Browse the repository at this point in the history
* fix: DataFusion suggests invalid functions

* update test

* Add test for BuiltInWindowFunction
  • Loading branch information
jonahgao authored Nov 8, 2023
1 parent 724bafd commit 3446382
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 6 deletions.
31 changes: 26 additions & 5 deletions datafusion/expr/src/aggregate_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,13 @@ impl AggregateFunction {
ArrayAgg => "ARRAY_AGG",
FirstValue => "FIRST_VALUE",
LastValue => "LAST_VALUE",
Variance => "VARIANCE",
VariancePop => "VARIANCE_POP",
Variance => "VAR",
VariancePop => "VAR_POP",
Stddev => "STDDEV",
StddevPop => "STDDEV_POP",
Covariance => "COVARIANCE",
CovariancePop => "COVARIANCE_POP",
Correlation => "CORRELATION",
Covariance => "COVAR",
CovariancePop => "COVAR_POP",
Correlation => "CORR",
RegrSlope => "REGR_SLOPE",
RegrIntercept => "REGR_INTERCEPT",
RegrCount => "REGR_COUNT",
Expand Down Expand Up @@ -411,3 +411,24 @@ impl AggregateFunction {
}
}
}

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

#[test]
// Test for AggregateFuncion's Display and from_str() implementations.
// For each variant in AggregateFuncion, it converts the variant to a string
// and then back to a variant. The test asserts that the original variant and
// the reconstructed variant are the same. This assertion is also necessary for
// function suggestion. See https://github.com/apache/arrow-datafusion/issues/8082
fn test_display_and_from_str() {
for func_original in AggregateFunction::iter() {
let func_name = func_original.to_string();
let func_from_str =
AggregateFunction::from_str(func_name.to_lowercase().as_str()).unwrap();
assert_eq!(func_from_str, func_original);
}
}
}
3 changes: 2 additions & 1 deletion datafusion/expr/src/built_in_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1621,7 +1621,8 @@ mod tests {
// Test for BuiltinScalarFunction's Display and from_str() implementations.
// For each variant in BuiltinScalarFunction, it converts the variant to a string
// and then back to a variant. The test asserts that the original variant and
// the reconstructed variant are the same.
// the reconstructed variant are the same. This assertion is also necessary for
// function suggestion. See https://github.com/apache/arrow-datafusion/issues/8082
fn test_display_and_from_str() {
for (_, func_original) in name_to_function().iter() {
let func_name = func_original.to_string();
Expand Down
15 changes: 15 additions & 0 deletions datafusion/expr/src/window_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ impl BuiltInWindowFunction {
#[cfg(test)]
mod tests {
use super::*;
use strum::IntoEnumIterator;

#[test]
fn test_count_return_type() -> Result<()> {
Expand Down Expand Up @@ -447,4 +448,18 @@ mod tests {
);
assert_eq!(find_df_window_func("not_exist"), None)
}

#[test]
// Test for BuiltInWindowFunction's Display and from_str() implementations.
// For each variant in BuiltInWindowFunction, it converts the variant to a string
// and then back to a variant. The test asserts that the original variant and
// the reconstructed variant are the same. This assertion is also necessary for
// function suggestion. See https://github.com/apache/arrow-datafusion/issues/8082
fn test_display_and_from_str() {
for func_original in BuiltInWindowFunction::iter() {
let func_name = func_original.to_string();
let func_from_str = BuiltInWindowFunction::from_str(&func_name).unwrap();
assert_eq!(func_from_str, func_original);
}
}
}
4 changes: 4 additions & 0 deletions datafusion/sqllogictest/test_files/functions.slt
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,10 @@ SELECT counter(*) from test;
statement error Did you mean 'STDDEV'?
SELECT STDEV(v1) from test;

# Aggregate function
statement error Did you mean 'COVAR'?
SELECT COVARIA(1,1);

# Window function
statement error Did you mean 'SUM'?
SELECT v1, v2, SUMM(v2) OVER(ORDER BY v1) from test;
Expand Down

0 comments on commit 3446382

Please sign in to comment.