diff --git a/e2e_test/udf/sql_udf.slt b/e2e_test/udf/sql_udf.slt index ed1a5ea327003..866f27abd52ce 100644 --- a/e2e_test/udf/sql_udf.slt +++ b/e2e_test/udf/sql_udf.slt @@ -332,12 +332,49 @@ In SQL UDF definition: `select i_am_valid + _this_is_invalid_please_properly_mar ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +# sql udf with non-existent function +statement error +create function call_non_existent_function(INT) returns int language sql as 'select 1 + non_existent(1) + 1'; +---- +db error: ERROR: Failed to run the query + +Caused by: + Invalid input syntax: Failed to conduct semantic check +function non_existent(integer) does not exist +In SQL UDF definition: `select 1 + non_existent(1) + 1` + ^^^^^^^^^^^^ + + +# First display invalid parameter +# if not found, try displaying non-existent functions +# if still not found, display default error message without hint +statement error +create function param_func_mix(a INT, b INT) returns int language sql as 'select a + b + c + not_be_displayed(c)'; +---- +db error: ERROR: Failed to run the query + +Caused by: + Invalid input syntax: Failed to conduct semantic check +Bind error: [sql udf] failed to find named parameter c +In SQL UDF definition: `select a + b + c + not_be_displayed(c)` + ^ + + statement error Expected end of statement, found: 💩 create function call_regexp_replace() returns varchar language sql as 'select regexp_replace('💩💩💩💩💩foo🤔️bar亲爱的😭baz这不是爱情❤️‍🔥', 'baz(...)', '这是🥵', 'ic')'; # Recursive definition can NOT be accepted at present due to semantic check -statement error Failed to conduct semantic check +statement error create function recursive(INT, INT) returns int language sql as 'select recursive($1, $2) + recursive($1, $2)'; +---- +db error: ERROR: Failed to run the query + +Caused by: + Invalid input syntax: Failed to conduct semantic check +function recursive(integer, integer) does not exist +In SQL UDF definition: `select recursive($1, $2) + recursive($1, $2)` + ^^^^^^^^^ + # Complex but error-prone definition, recursive & normal sql udfs interleaving statement error Failed to conduct semantic check diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index 71fb480ae9ab3..f0ab16a4be540 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -32,31 +32,56 @@ use crate::catalog::CatalogError; use crate::expr::{Expr, ExprImpl, Literal}; use crate::{bind_data_type, Binder}; +/// The error type for hint display +/// Currently we will try invalid parameter first +/// Then try to find non-existent functions +enum ErrMsgType { + Parameter, + Function, + // Not yet support + None, +} + const DEFAULT_ERR_MSG: &str = "Failed to conduct semantic check"; +/// Used for hint display const PROMPT: &str = "In SQL UDF definition: "; -pub const SQL_UDF_PATTERN: &str = "[sql udf]"; +/// Used for detecting non-existent function +const FUNCTION_KEYWORD: &str = "function"; -/// Create a mock `udf_context`, which is used for semantic check -fn create_mock_udf_context( - arg_types: Vec, - arg_names: Vec, -) -> HashMap { - let mut ret: HashMap = (1..=arg_types.len()) - .map(|i| { - let mock_expr = - ExprImpl::Literal(Box::new(Literal::new(None, arg_types[i - 1].clone()))); - (format!("${i}"), mock_expr) - }) - .collect(); +/// Used for detecting invalid parameters +pub const SQL_UDF_PATTERN: &str = "[sql udf]"; - for (i, arg_name) in arg_names.into_iter().enumerate() { - let mock_expr = ExprImpl::Literal(Box::new(Literal::new(None, arg_types[i].clone()))); - ret.insert(arg_name, mock_expr); +/// Validate the error message to see if +/// it's possible to improve the display to users +fn validate_err_msg(invalid_msg: &str) -> ErrMsgType { + // First try invalid parameters + if invalid_msg.contains(SQL_UDF_PATTERN) { + ErrMsgType::Parameter + } else if invalid_msg.contains(FUNCTION_KEYWORD) { + ErrMsgType::Function + } else { + // Nothing could be better display + ErrMsgType::None } +} - ret +/// Extract the target name to hint display +/// according to the type of the error message item +fn extract_hint_display_target(err_msg_type: ErrMsgType, invalid_msg: &str) -> Option<&str> { + match err_msg_type { + // e.g., [sql udf] failed to find named parameter + ErrMsgType::Parameter => invalid_msg.split_whitespace().last(), + // e.g., function does not exist + ErrMsgType::Function => { + let func = invalid_msg.split_whitespace().nth(1).unwrap_or("null"); + // Note: we do not want the parenthesis + func.find('(').map(|i| &func[0..i]) + } + // Nothing to hint display, return default error message + ErrMsgType::None => None, + } } /// Find the pattern for better hint display @@ -77,6 +102,27 @@ fn find_target(input: &str, target: &str) -> Option { Some(ma.start()) } +/// Create a mock `udf_context`, which is used for semantic check +fn create_mock_udf_context( + arg_types: Vec, + arg_names: Vec, +) -> HashMap { + let mut ret: HashMap = (1..=arg_types.len()) + .map(|i| { + let mock_expr = + ExprImpl::Literal(Box::new(Literal::new(None, arg_types[i - 1].clone()))); + (format!("${i}"), mock_expr) + }) + .collect(); + + for (i, arg_name) in arg_names.into_iter().enumerate() { + let mock_expr = ExprImpl::Literal(Box::new(Literal::new(None, arg_types[i].clone()))); + ret.insert(arg_name, mock_expr); + } + + ret +} + pub async fn handle_create_sql_function( handler_args: HandlerArgs, or_replace: bool, @@ -233,19 +279,20 @@ pub async fn handle_create_sql_function( if let ErrorCode::BindErrorRoot { expr: _, error } = e.inner() { let invalid_msg = error.to_string(); - // Valid error message for hint display - let Some(_) = invalid_msg.as_str().find(SQL_UDF_PATTERN) else { + // First validate the message + let err_msg_type = validate_err_msg(invalid_msg.as_str()); + + // Get the name of the invalid item + // We will just display the first one found + let Some(invalid_item_name) = + extract_hint_display_target(err_msg_type, invalid_msg.as_str()) + else { return Err( ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into() ); }; - // Get the name of the invalid item - // We will just display the first one found - let invalid_item_name = - invalid_msg.split_whitespace().last().unwrap_or("null"); - - // Find the invalid parameter / column + // Find the invalid parameter / column / function let Some(idx) = find_target(body.as_str(), invalid_item_name) else { return Err( ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into()