From aeca7ea99940401573eb14601c74316279bbdae5 Mon Sep 17 00:00:00 2001
From: Andrew Lamb <andrew@nerdnetworks.org>
Date: Tue, 17 Sep 2024 15:54:13 -0400
Subject: [PATCH] Minor: use Option rather than Result for not found suggestion
 (#12512)

---
 datafusion/sql/src/expr/function.rs | 38 +++++++++++++++--------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/datafusion/sql/src/expr/function.rs b/datafusion/sql/src/expr/function.rs
index 1c114523d71e..ddafc4e3a03a 100644
--- a/datafusion/sql/src/expr/function.rs
+++ b/datafusion/sql/src/expr/function.rs
@@ -19,8 +19,8 @@ use crate::planner::{ContextProvider, PlannerContext, SqlToRel};
 
 use arrow_schema::DataType;
 use datafusion_common::{
-    internal_datafusion_err, not_impl_err, plan_datafusion_err, plan_err, DFSchema,
-    Dependency, Result,
+    internal_datafusion_err, internal_err, not_impl_err, plan_datafusion_err, plan_err,
+    DFSchema, Dependency, Result,
 };
 use datafusion_expr::expr::WildcardOptions;
 use datafusion_expr::planner::PlannerResult;
@@ -39,11 +39,14 @@ use sqlparser::ast::{
 use strum::IntoEnumIterator;
 
 /// Suggest a valid function based on an invalid input function name
+///
+/// Returns `None` if no valid matches are found. This happens when there are no
+/// functions registered with the context.
 pub fn suggest_valid_function(
     input_function_name: &str,
     is_window_func: bool,
     ctx: &dyn ContextProvider,
-) -> Result<String> {
+) -> Option<String> {
     let valid_funcs = if is_window_func {
         // All aggregate functions and builtin window functions
         let mut funcs = Vec::new();
@@ -67,19 +70,14 @@ pub fn suggest_valid_function(
 
 /// Find the closest matching string to the target string in the candidates list, using edit distance(case insensitive)
 /// Input `candidates` must not be empty otherwise an error is returned.
-fn find_closest_match(candidates: Vec<String>, target: &str) -> Result<String> {
+fn find_closest_match(candidates: Vec<String>, target: &str) -> Option<String> {
     let target = target.to_lowercase();
-    candidates
-        .into_iter()
-        .min_by_key(|candidate| {
-            datafusion_common::utils::datafusion_strsim::levenshtein(
-                &candidate.to_lowercase(),
-                &target,
-            )
-        })
-        .ok_or_else(|| {
-            internal_datafusion_err!("No functions registered with this context.")
-        })
+    candidates.into_iter().min_by_key(|candidate| {
+        datafusion_common::utils::datafusion_strsim::levenshtein(
+            &candidate.to_lowercase(),
+            &target,
+        )
+    })
 }
 
 /// Arguments to for a function call extracted from the SQL AST
@@ -355,9 +353,13 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
         }
 
         // Could not find the relevant function, so return an error
-        let suggested_func_name =
-            suggest_valid_function(&name, is_function_window, self.context_provider)?;
-        plan_err!("Invalid function '{name}'.\nDid you mean '{suggested_func_name}'?")
+        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}'?")
+        } else {
+            internal_err!("No functions registered with this context.")
+        }
     }
 
     pub(super) fn sql_fn_name_to_expr(