Skip to content

Commit

Permalink
feat(sql-udf): add hint display for non-existent functions (#15188)
Browse files Browse the repository at this point in the history
Co-authored-by: xzhseh <[email protected]>
  • Loading branch information
xzhseh and xzhseh authored Feb 29, 2024
1 parent 07856b8 commit 463cfaf
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 26 deletions.
39 changes: 38 additions & 1 deletion e2e_test/udf/sql_udf.slt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
97 changes: 72 additions & 25 deletions src/frontend/src/handler/create_sql_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<DataType>,
arg_names: Vec<String>,
) -> HashMap<String, ExprImpl> {
let mut ret: HashMap<String, ExprImpl> = (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 <target name>
ErrMsgType::Parameter => invalid_msg.split_whitespace().last(),
// e.g., function <target name> 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
Expand All @@ -77,6 +102,27 @@ fn find_target(input: &str, target: &str) -> Option<usize> {
Some(ma.start())
}

/// Create a mock `udf_context`, which is used for semantic check
fn create_mock_udf_context(
arg_types: Vec<DataType>,
arg_names: Vec<String>,
) -> HashMap<String, ExprImpl> {
let mut ret: HashMap<String, ExprImpl> = (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,
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 463cfaf

Please sign in to comment.