Skip to content

fix: DataFusion suggests invalid functions #8083

New issue

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

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

Already on GitHub? # to your account

Merged
merged 3 commits into from
Nov 8, 2023
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
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() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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