diff --git a/datafusion/sql/src/expr/function.rs b/datafusion/sql/src/expr/function.rs index 1cf3dcb289a6..e9bf083f3279 100644 --- a/datafusion/sql/src/expr/function.rs +++ b/datafusion/sql/src/expr/function.rs @@ -20,7 +20,7 @@ use crate::planner::{ContextProvider, PlannerContext, SqlToRel}; use arrow::datatypes::DataType; use datafusion_common::{ internal_datafusion_err, internal_err, not_impl_err, plan_datafusion_err, plan_err, - DFSchema, Dependency, Result, + DFSchema, Dependency, Diagnostic, Result, Span, }; use datafusion_expr::expr::{ScalarFunction, Unnest}; use datafusion_expr::planner::PlannerResult; @@ -217,7 +217,7 @@ impl SqlToRel<'_, S> { // it shouldn't have ordering requirement as function argument // required ordering should be defined in OVER clause. let is_function_window = over.is_some(); - + let sql_parser_span = name.0[0].span; let name = if name.0.len() > 1 { // DF doesn't handle compound identifiers // (e.g. "foo.bar") for function names yet @@ -236,7 +236,6 @@ impl SqlToRel<'_, S> { } } } - // User-defined function (UDF) should have precedence if let Some(fm) = self.context_provider.get_function_meta(&name) { let args = self.function_args_to_expr(args, schema, planner_context)?; @@ -259,7 +258,6 @@ impl SqlToRel<'_, S> { "Aggregate ORDER BY is not implemented for window functions" ); } - // Then, window function if let Some(WindowType::WindowSpec(window)) = over { let partition_by = window @@ -351,12 +349,21 @@ impl SqlToRel<'_, S> { ))); } } - // Could not find the relevant function, so return an error if let Some(suggested_func_name) = suggest_valid_function(&name, is_function_window, self.context_provider) { plan_err!("Invalid function '{name}'.\nDid you mean '{suggested_func_name}'?") + .map_err(|e| { + let span = Span::try_from_sqlparser_span(sql_parser_span); + let mut diagnostic = + Diagnostic::new_error(format!("Invalid function '{name}'"), span); + diagnostic.add_note( + format!("Possible function '{}'", suggested_func_name), + None, + ); + e.with_diagnostic(diagnostic) + }) } else { internal_err!("No functions registered with this context.") } diff --git a/datafusion/sql/tests/cases/diagnostic.rs b/datafusion/sql/tests/cases/diagnostic.rs index 9dae2d0c3e93..d70484f718c8 100644 --- a/datafusion/sql/tests/cases/diagnostic.rs +++ b/datafusion/sql/tests/cases/diagnostic.rs @@ -15,7 +15,8 @@ // specific language governing permissions and limitations // under the License. -use std::collections::HashMap; +use datafusion_functions::string; +use std::{collections::HashMap, sync::Arc}; use datafusion_common::{Diagnostic, Location, Result, Span}; use datafusion_sql::planner::{ParserOptions, SqlToRel}; @@ -35,8 +36,8 @@ fn do_query(sql: &'static str) -> Diagnostic { collect_spans: true, ..ParserOptions::default() }; - - let state = MockSessionState::default(); + let state = MockSessionState::default() + .with_scalar_function(Arc::new(string::concat().as_ref().clone())); let context = MockContextProvider { state }; let sql_to_rel = SqlToRel::new_with_options(&context, options); match sql_to_rel.sql_statement_to_plan(statement) { @@ -274,3 +275,14 @@ fn test_ambiguous_column_suggestion() -> Result<()> { Ok(()) } + +#[test] +fn test_invalid_function() -> Result<()> { + let query = "SELECT /*whole*/concat_not_exist/*whole*/()"; + let spans = get_spans(query); + let diag = do_query(query); + assert_eq!(diag.message, "Invalid function 'concat_not_exist'"); + assert_eq!(diag.notes[0].message, "Possible function 'concat'"); + assert_eq!(diag.span, Some(spans["whole"])); + Ok(()) +}