Skip to content

Commit aeca7ea

Browse files
authored
Minor: use Option rather than Result for not found suggestion (#12512)
1 parent 8555e41 commit aeca7ea

File tree

1 file changed

+20
-18
lines changed

1 file changed

+20
-18
lines changed

datafusion/sql/src/expr/function.rs

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ use crate::planner::{ContextProvider, PlannerContext, SqlToRel};
1919

2020
use arrow_schema::DataType;
2121
use datafusion_common::{
22-
internal_datafusion_err, not_impl_err, plan_datafusion_err, plan_err, DFSchema,
23-
Dependency, Result,
22+
internal_datafusion_err, internal_err, not_impl_err, plan_datafusion_err, plan_err,
23+
DFSchema, Dependency, Result,
2424
};
2525
use datafusion_expr::expr::WildcardOptions;
2626
use datafusion_expr::planner::PlannerResult;
@@ -39,11 +39,14 @@ use sqlparser::ast::{
3939
use strum::IntoEnumIterator;
4040

4141
/// Suggest a valid function based on an invalid input function name
42+
///
43+
/// Returns `None` if no valid matches are found. This happens when there are no
44+
/// functions registered with the context.
4245
pub fn suggest_valid_function(
4346
input_function_name: &str,
4447
is_window_func: bool,
4548
ctx: &dyn ContextProvider,
46-
) -> Result<String> {
49+
) -> Option<String> {
4750
let valid_funcs = if is_window_func {
4851
// All aggregate functions and builtin window functions
4952
let mut funcs = Vec::new();
@@ -67,19 +70,14 @@ pub fn suggest_valid_function(
6770

6871
/// Find the closest matching string to the target string in the candidates list, using edit distance(case insensitive)
6972
/// Input `candidates` must not be empty otherwise an error is returned.
70-
fn find_closest_match(candidates: Vec<String>, target: &str) -> Result<String> {
73+
fn find_closest_match(candidates: Vec<String>, target: &str) -> Option<String> {
7174
let target = target.to_lowercase();
72-
candidates
73-
.into_iter()
74-
.min_by_key(|candidate| {
75-
datafusion_common::utils::datafusion_strsim::levenshtein(
76-
&candidate.to_lowercase(),
77-
&target,
78-
)
79-
})
80-
.ok_or_else(|| {
81-
internal_datafusion_err!("No functions registered with this context.")
82-
})
75+
candidates.into_iter().min_by_key(|candidate| {
76+
datafusion_common::utils::datafusion_strsim::levenshtein(
77+
&candidate.to_lowercase(),
78+
&target,
79+
)
80+
})
8381
}
8482

8583
/// Arguments to for a function call extracted from the SQL AST
@@ -355,9 +353,13 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
355353
}
356354

357355
// Could not find the relevant function, so return an error
358-
let suggested_func_name =
359-
suggest_valid_function(&name, is_function_window, self.context_provider)?;
360-
plan_err!("Invalid function '{name}'.\nDid you mean '{suggested_func_name}'?")
356+
if let Some(suggested_func_name) =
357+
suggest_valid_function(&name, is_function_window, self.context_provider)
358+
{
359+
plan_err!("Invalid function '{name}'.\nDid you mean '{suggested_func_name}'?")
360+
} else {
361+
internal_err!("No functions registered with this context.")
362+
}
361363
}
362364

363365
pub(super) fn sql_fn_name_to_expr(

0 commit comments

Comments
 (0)