chore: Attach Diagnostic to "function x does not exist" error (#14849)

* Add detailed error message for invalid function error

* remove clone

* better note

* fix test case

Author: onlyjackfrost

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, RawAggregateExpr, RawWindowExpr};
@@ -217,7 +217,7 @@ impl<S: ContextProvider> 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<S: ContextProvider> 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<S: ContextProvider> 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
@@ -399,12 +397,21 @@ impl<S: ContextProvider> 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.")
         }

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(())
+}