From f07dd5b1292fc8c160245d5cc7c0437d75c9b8db Mon Sep 17 00:00:00 2001 From: Zihao Xu Date: Wed, 14 Feb 2024 03:53:39 +0800 Subject: [PATCH 01/25] better display --- Cargo.lock | 13 +++- e2e_test/udf/sql_udf.slt | 8 +-- src/frontend/Cargo.toml | 1 + src/frontend/src/binder/expr/column.rs | 4 +- src/frontend/src/binder/expr/mod.rs | 7 ++ .../src/handler/create_sql_function.rs | 66 +++++++++++++++++-- 6 files changed, 87 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8f91a6aa97020..fcd94df87e8ad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3885,6 +3885,16 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2acce4a10f12dc2fb14a218589d4f1f62ef011b2d0cc4b3cb1bba8e94da14649" +[[package]] +name = "fancy-regex" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b95f7c0680e4142284cf8b22c14a476e87d61b004a3a0861872b32ef7ead40a2" +dependencies = [ + "bit-set", + "regex", +] + [[package]] name = "fancy-regex" version = "0.13.0" @@ -9211,7 +9221,7 @@ dependencies = [ "chrono", "criterion", "expect-test", - "fancy-regex", + "fancy-regex 0.13.0", "futures-async-stream", "futures-util", "hex", @@ -9273,6 +9283,7 @@ dependencies = [ "educe 0.5.7", "either", "enum-as-inner", + "fancy-regex 0.11.0", "fixedbitset", "futures", "futures-async-stream", diff --git a/e2e_test/udf/sql_udf.slt b/e2e_test/udf/sql_udf.slt index c33daba9d7ccf..d2a25f7997d11 100644 --- a/e2e_test/udf/sql_udf.slt +++ b/e2e_test/udf/sql_udf.slt @@ -287,16 +287,16 @@ 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, please see if you are calling non-existent functions +statement error failed to conduct semantic check create function recursive(INT, INT) returns int language sql as 'select recursive($1, $2) + recursive($1, $2)'; # Complex but error-prone definition, recursive & normal sql udfs interleaving -statement error failed to conduct semantic check, please see if you are calling non-existent functions +statement error failed to conduct semantic check create function recursive_non_recursive(INT, INT) returns int language sql as 'select recursive($1, $2) + sub($1, $2)'; # Create a valid recursive function # Please note we do NOT support actual running the recursive sql udf at present -statement error failed to conduct semantic check, please see if you are calling non-existent functions +statement error failed to conduct semantic check create function fib(INT) returns int language sql as 'select case when $1 = 0 then 0 @@ -307,7 +307,7 @@ create function fib(INT) returns int end;'; # Calling a non-existent function -statement error failed to conduct semantic check, please see if you are calling non-existent functions +statement error failed to conduct semantic check create function non_exist(INT) returns int language sql as 'select yo(114514)'; # Try to create an sql udf with unnamed parameters whose return type mismatches with the sql body definition diff --git a/src/frontend/Cargo.toml b/src/frontend/Cargo.toml index f6254f2a4b172..0870fe8d9e7c1 100644 --- a/src/frontend/Cargo.toml +++ b/src/frontend/Cargo.toml @@ -91,6 +91,7 @@ tokio-stream = "0.1" tonic = { workspace = true } tracing = "0.1" uuid = "1" +fancy-regex = "0.11.0" [target.'cfg(not(madsim))'.dependencies] workspace-hack = { path = "../workspace-hack" } diff --git a/src/frontend/src/binder/expr/column.rs b/src/frontend/src/binder/expr/column.rs index 29f1a0f1c64e5..369dcc4e2d2b3 100644 --- a/src/frontend/src/binder/expr/column.rs +++ b/src/frontend/src/binder/expr/column.rs @@ -54,8 +54,10 @@ impl Binder { // there will not exist any column identifiers // And invalid cases should already be caught // during semantic check phase + // Note: the error message here also help with hint display + // when invalid definition occurs at sql udf creation time return Err(ErrorCode::BindError(format!( - "failed to find named parameter {column_name}" + "[sql udf] failed to find named parameter {column_name}" )) .into()); } diff --git a/src/frontend/src/binder/expr/mod.rs b/src/frontend/src/binder/expr/mod.rs index 2baf9cb1f84e7..ebdc6c8679877 100644 --- a/src/frontend/src/binder/expr/mod.rs +++ b/src/frontend/src/binder/expr/mod.rs @@ -392,6 +392,13 @@ impl Binder { if self.udf_context.global_count() != 0 { if let Some(expr) = self.udf_context.get_expr(&format!("${index}")) { return Ok(expr.clone()); + } else { + // Same as `bind_column`, the error message here + // help with hint display when invalid definition occurs + return Err(ErrorCode::BindError(format!( + "[sql udf] failed to find unnamed parameter ${index}" + )) + .into()); } } diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index 311664735603f..3aeccbb833071 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::collections::HashMap; +use fancy_regex::Regex; use itertools::Itertools; use pgwire::pg_response::StatementType; @@ -24,7 +25,6 @@ use risingwave_sqlparser::ast::{ CreateFunctionBody, FunctionDefinition, ObjectName, OperateFunctionArg, }; use risingwave_sqlparser::parser::{Parser, ParserError}; -use thiserror_ext::AsReport; use super::*; use crate::binder::UdfContext; @@ -53,6 +53,23 @@ fn create_mock_udf_context( ret } +/// Find the pattern for better hint display +/// return the exact index where the pattern first appears +fn find_target(input: &str, target: &str) -> Option { + // Regex pattern to find `target` not preceded/followed by an ASCII letter. + // \b asserts a word boundary, and \B asserts NOT a word boundary + // The pattern uses negative lookbehind (? return Err(ErrorCode::InvalidInputSyntax(format!( - "failed to conduct semantic check, please see if you are calling non-existence functions or parameters\ndetailed error message: {}", - e.as_report() - )) - .into()), + Err(e) => { + let default_err_msg = "Failed to conduct semantic check".to_string(); + let prompt = "In SQL UDF definition: ".to_string(); + + if let ErrorCode::BindErrorRoot { expr: _, error } = e.inner() { + let invalid_msg = error.to_string(); + let pattern = "[sql udf]"; + + // Valid error message for hint display + let Some(_) = invalid_msg.as_str().find(pattern) else { + return Err(ErrorCode::InvalidInputSyntax(default_err_msg).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 + let Some(idx) = find_target(body.as_str(), invalid_item_name) else { + return Err(ErrorCode::InvalidInputSyntax(default_err_msg).into()); + }; + + println!("Current idx: {}", idx); + + // The exact error position for `^` to point to + let position = format!("{}{}", + " ".repeat(idx + prompt.len() + 1), + "^".repeat(invalid_item_name.len())); + + return Err(ErrorCode::InvalidInputSyntax( + format!("\n{}: {}\n{}`{}`\n{}", + default_err_msg, + invalid_msg, + prompt, + body, + position) + ).into()); + } + + // Otherwise return the default error message + return Err(ErrorCode::InvalidInputSyntax(default_err_msg).into()) + } } } else { return Err(ErrorCode::InvalidInputSyntax( From 876f81000da6ac38c5f379f04975316c4f4c5038 Mon Sep 17 00:00:00 2001 From: Zihao Xu Date: Wed, 14 Feb 2024 03:59:44 +0800 Subject: [PATCH 02/25] make default_err_msg & prompt constant --- .../src/handler/create_sql_function.rs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index 3aeccbb833071..9774652c58c0f 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -32,6 +32,10 @@ use crate::catalog::CatalogError; use crate::expr::{Expr, ExprImpl, Literal}; use crate::{bind_data_type, Binder}; +const DEFAULT_ERR_MSG: &str = "Failed to conduct semantic check"; + +const PROMPT: &str = "In SQL UDF definition: "; + /// Create a mock `udf_context`, which is used for semantic check fn create_mock_udf_context( arg_types: Vec, @@ -223,8 +227,6 @@ pub async fn handle_create_sql_function( } } Err(e) => { - let default_err_msg = "Failed to conduct semantic check".to_string(); - let prompt = "In SQL UDF definition: ".to_string(); if let ErrorCode::BindErrorRoot { expr: _, error } = e.inner() { let invalid_msg = error.to_string(); @@ -232,7 +234,7 @@ pub async fn handle_create_sql_function( // Valid error message for hint display let Some(_) = invalid_msg.as_str().find(pattern) else { - return Err(ErrorCode::InvalidInputSyntax(default_err_msg).into()); + return Err(ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into()); }; // Get the name of the invalid item @@ -241,28 +243,26 @@ pub async fn handle_create_sql_function( // Find the invalid parameter / column let Some(idx) = find_target(body.as_str(), invalid_item_name) else { - return Err(ErrorCode::InvalidInputSyntax(default_err_msg).into()); + return Err(ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into()); }; - println!("Current idx: {}", idx); - // The exact error position for `^` to point to let position = format!("{}{}", - " ".repeat(idx + prompt.len() + 1), + " ".repeat(idx + PROMPT.len() + 1), "^".repeat(invalid_item_name.len())); return Err(ErrorCode::InvalidInputSyntax( format!("\n{}: {}\n{}`{}`\n{}", - default_err_msg, + DEFAULT_ERR_MSG, invalid_msg, - prompt, + PROMPT, body, position) ).into()); } // Otherwise return the default error message - return Err(ErrorCode::InvalidInputSyntax(default_err_msg).into()) + return Err(ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into()) } } } else { From fbf809c2ef0f7b8fb87ba2c157fe30d749f9974a Mon Sep 17 00:00:00 2001 From: xzhseh Date: Tue, 13 Feb 2024 20:04:04 +0000 Subject: [PATCH 03/25] Fix "cargo-hakari" --- Cargo.lock | 1 + src/workspace-hack/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index fcd94df87e8ad..d79ba98df2e9b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13730,6 +13730,7 @@ dependencies = [ "axum", "base64 0.21.4", "bigdecimal 0.4.2", + "bit-set", "bit-vec", "bitflags 2.4.0", "byteorder", diff --git a/src/workspace-hack/Cargo.toml b/src/workspace-hack/Cargo.toml index 0c7df07fa5b79..a4be6e65e82c2 100644 --- a/src/workspace-hack/Cargo.toml +++ b/src/workspace-hack/Cargo.toml @@ -32,6 +32,7 @@ aws-smithy-types = { version = "1", default-features = false, features = ["byte- axum = { version = "0.6" } base64 = { version = "0.21", features = ["alloc"] } bigdecimal = { version = "0.4" } +bit-set = { version = "0.5" } bit-vec = { version = "0.6" } bitflags = { version = "2", default-features = false, features = ["serde", "std"] } byteorder = { version = "1" } From d9f9360c3f2f0e1d408587ffa3eb7cf70bf7bb7a Mon Sep 17 00:00:00 2001 From: Zihao Xu Date: Wed, 14 Feb 2024 04:08:27 +0800 Subject: [PATCH 04/25] fix check --- src/frontend/Cargo.toml | 2 +- .../src/handler/create_sql_function.rs | 41 ++++++++++--------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/frontend/Cargo.toml b/src/frontend/Cargo.toml index 0870fe8d9e7c1..b3e18a1e78380 100644 --- a/src/frontend/Cargo.toml +++ b/src/frontend/Cargo.toml @@ -33,6 +33,7 @@ easy-ext = "1" educe = "0.5" either = "1" enum-as-inner = "0.6" +fancy-regex = "0.11.0" fixedbitset = "0.4.2" futures = { version = "0.3", default-features = false, features = ["alloc"] } futures-async-stream = { workspace = true } @@ -91,7 +92,6 @@ tokio-stream = "0.1" tonic = { workspace = true } tracing = "0.1" uuid = "1" -fancy-regex = "0.11.0" [target.'cfg(not(madsim))'.dependencies] workspace-hack = { path = "../workspace-hack" } diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index 9774652c58c0f..88554328b4cb8 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -13,8 +13,8 @@ // limitations under the License. use std::collections::HashMap; -use fancy_regex::Regex; +use fancy_regex::Regex; use itertools::Itertools; use pgwire::pg_response::StatementType; use risingwave_common::catalog::FunctionId; @@ -227,42 +227,45 @@ pub async fn handle_create_sql_function( } } Err(e) => { - if let ErrorCode::BindErrorRoot { expr: _, error } = e.inner() { let invalid_msg = error.to_string(); let pattern = "[sql udf]"; // Valid error message for hint display let Some(_) = invalid_msg.as_str().find(pattern) else { - return Err(ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into()); + 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"); + let invalid_item_name = + invalid_msg.split_whitespace().last().unwrap_or("null"); // Find the invalid parameter / column let Some(idx) = find_target(body.as_str(), invalid_item_name) else { - return Err(ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into()); + return Err( + ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into() + ); }; // The exact error position for `^` to point to - let position = format!("{}{}", + let position = format!( + "{}{}", " ".repeat(idx + PROMPT.len() + 1), - "^".repeat(invalid_item_name.len())); - - return Err(ErrorCode::InvalidInputSyntax( - format!("\n{}: {}\n{}`{}`\n{}", - DEFAULT_ERR_MSG, - invalid_msg, - PROMPT, - body, - position) - ).into()); - } + "^".repeat(invalid_item_name.len()) + ); - // Otherwise return the default error message - return Err(ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into()) + Err(ErrorCode::InvalidInputSyntax(format!( + "\n{}: {}\n{}`{}`\n{}", + DEFAULT_ERR_MSG, invalid_msg, PROMPT, body, position + )) + .into()) + } else { + // Otherwise return the default error message + Err(ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into()); + } } } } else { From 5a2efaba405da8fa8ad58d8e16781f5e47bcc5e4 Mon Sep 17 00:00:00 2001 From: Zihao Xu Date: Wed, 14 Feb 2024 04:24:26 +0800 Subject: [PATCH 05/25] make pattern constant --- src/frontend/src/handler/create_sql_function.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index 88554328b4cb8..14902c437fcd0 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -36,6 +36,8 @@ const DEFAULT_ERR_MSG: &str = "Failed to conduct semantic check"; const PROMPT: &str = "In SQL UDF definition: "; +const SQL_UDF_PATTERN: &str = "[sql udf]"; + /// Create a mock `udf_context`, which is used for semantic check fn create_mock_udf_context( arg_types: Vec, @@ -229,10 +231,9 @@ pub async fn handle_create_sql_function( Err(e) => { if let ErrorCode::BindErrorRoot { expr: _, error } = e.inner() { let invalid_msg = error.to_string(); - let pattern = "[sql udf]"; // Valid error message for hint display - let Some(_) = invalid_msg.as_str().find(pattern) else { + let Some(_) = invalid_msg.as_str().find(SQL_UDF_PATTERN) else { return Err( ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into() ); @@ -257,14 +258,14 @@ pub async fn handle_create_sql_function( "^".repeat(invalid_item_name.len()) ); - Err(ErrorCode::InvalidInputSyntax(format!( + return Err(ErrorCode::InvalidInputSyntax(format!( "\n{}: {}\n{}`{}`\n{}", DEFAULT_ERR_MSG, invalid_msg, PROMPT, body, position )) - .into()) + .into()); } else { // Otherwise return the default error message - Err(ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into()); + return Err(ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into()); } } } From c31e28516a927b7ab3807de520f0cfec2c79b9e7 Mon Sep 17 00:00:00 2001 From: Zihao Xu Date: Wed, 14 Feb 2024 05:14:50 +0800 Subject: [PATCH 06/25] update sql udf test --- e2e_test/udf/sql_udf.slt | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/e2e_test/udf/sql_udf.slt b/e2e_test/udf/sql_udf.slt index d2a25f7997d11..21ccf414cfe1e 100644 --- a/e2e_test/udf/sql_udf.slt +++ b/e2e_test/udf/sql_udf.slt @@ -280,6 +280,17 @@ $1 + 114514 + $1 # Named sql udf with invalid parameter in body definition # Will be rejected at creation time +# P.S. Unfortunately it seems that sqllogictest-rs in RW does not support multi-line error yet +# The following error message is just for reference +# ---------------------------------------- +# 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 aa +# In SQL UDF definition: `select a + aa + a` +# ^^ +# ---------------------------------------- statement error failed to find named parameter aa create function unknown_parameter(a INT) returns int language sql as 'select a + aa + a'; @@ -287,16 +298,16 @@ 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 Failed to conduct semantic check create function recursive(INT, INT) returns int language sql as 'select recursive($1, $2) + recursive($1, $2)'; # Complex but error-prone definition, recursive & normal sql udfs interleaving -statement error failed to conduct semantic check +statement error Failed to conduct semantic check create function recursive_non_recursive(INT, INT) returns int language sql as 'select recursive($1, $2) + sub($1, $2)'; # Create a valid recursive function # Please note we do NOT support actual running the recursive sql udf at present -statement error failed to conduct semantic check +statement error Failed to conduct semantic check create function fib(INT) returns int language sql as 'select case when $1 = 0 then 0 @@ -307,7 +318,7 @@ create function fib(INT) returns int end;'; # Calling a non-existent function -statement error failed to conduct semantic check +statement error Failed to conduct semantic check create function non_exist(INT) returns int language sql as 'select yo(114514)'; # Try to create an sql udf with unnamed parameters whose return type mismatches with the sql body definition From 44781a4ba3c6fb541ca5ae5f3a7fa55cbf288d99 Mon Sep 17 00:00:00 2001 From: Zihao Xu Date: Wed, 14 Feb 2024 05:37:46 +0800 Subject: [PATCH 07/25] fix check --- e2e_test/udf/sql_udf.slt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/e2e_test/udf/sql_udf.slt b/e2e_test/udf/sql_udf.slt index 21ccf414cfe1e..10c81d8ce08db 100644 --- a/e2e_test/udf/sql_udf.slt +++ b/e2e_test/udf/sql_udf.slt @@ -284,9 +284,9 @@ $1 + 114514 + $1 # The following error message is just for reference # ---------------------------------------- # db error: ERROR: Failed to run the query -# +# # Caused by: -# Invalid input syntax: +# Invalid input syntax: # Failed to conduct semantic check: Bind error: [sql udf] failed to find named parameter aa # In SQL UDF definition: `select a + aa + a` # ^^ From a390d3dc09f7aa809f1e1e9718dddfaa9c4a0bb0 Mon Sep 17 00:00:00 2001 From: Zihao Xu Date: Wed, 14 Feb 2024 09:59:50 +0800 Subject: [PATCH 08/25] adjust the display a little bit --- src/frontend/src/handler/create_sql_function.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index 14902c437fcd0..1fcdeb884b42b 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -259,7 +259,7 @@ pub async fn handle_create_sql_function( ); return Err(ErrorCode::InvalidInputSyntax(format!( - "\n{}: {}\n{}`{}`\n{}", + "{}\n{}\n{}`{}`\n{}", DEFAULT_ERR_MSG, invalid_msg, PROMPT, body, position )) .into()); From bbf48a69ba6c98b400e6c642e5939d3f14e76855 Mon Sep 17 00:00:00 2001 From: Zihao Xu Date: Wed, 14 Feb 2024 10:36:20 +0800 Subject: [PATCH 09/25] tiny update --- e2e_test/udf/sql_udf.slt | 11 ++++++++++- src/frontend/src/handler/create_sql_function.rs | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/e2e_test/udf/sql_udf.slt b/e2e_test/udf/sql_udf.slt index 10c81d8ce08db..a6c0e7c1b916b 100644 --- a/e2e_test/udf/sql_udf.slt +++ b/e2e_test/udf/sql_udf.slt @@ -291,8 +291,17 @@ $1 + 114514 + $1 # In SQL UDF definition: `select a + aa + a` # ^^ # ---------------------------------------- -statement error failed to find named parameter aa +# statement error failed to find named parameter aa +statement error create function unknown_parameter(a INT) returns int language sql as 'select a + aa + a'; +---- +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 aa +In SQL UDF definition: `select a + aa + a` + ^^ 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')'; diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index 1fcdeb884b42b..9f6475dad53dd 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -259,7 +259,7 @@ pub async fn handle_create_sql_function( ); return Err(ErrorCode::InvalidInputSyntax(format!( - "{}\n{}\n{}`{}`\n{}", + "{}\n {}\n {}`{}`\n{}", DEFAULT_ERR_MSG, invalid_msg, PROMPT, body, position )) .into()); From 119f528b5f08c626f731ebe1c77828be39f0fee9 Mon Sep 17 00:00:00 2001 From: Zihao Xu Date: Wed, 14 Feb 2024 11:15:11 +0800 Subject: [PATCH 10/25] add multi-line error message --- e2e_test/udf/sql_udf.slt | 13 +------------ src/frontend/src/handler/create_sql_function.rs | 2 +- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/e2e_test/udf/sql_udf.slt b/e2e_test/udf/sql_udf.slt index a6c0e7c1b916b..a5780fb04f110 100644 --- a/e2e_test/udf/sql_udf.slt +++ b/e2e_test/udf/sql_udf.slt @@ -280,18 +280,6 @@ $1 + 114514 + $1 # Named sql udf with invalid parameter in body definition # Will be rejected at creation time -# P.S. Unfortunately it seems that sqllogictest-rs in RW does not support multi-line error yet -# The following error message is just for reference -# ---------------------------------------- -# 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 aa -# In SQL UDF definition: `select a + aa + a` -# ^^ -# ---------------------------------------- -# statement error failed to find named parameter aa statement error create function unknown_parameter(a INT) returns int language sql as 'select a + aa + a'; ---- @@ -303,6 +291,7 @@ Bind error: [sql udf] failed to find named parameter aa In SQL UDF definition: `select a + aa + a` ^^ + 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')'; diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index 9f6475dad53dd..1fcdeb884b42b 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -259,7 +259,7 @@ pub async fn handle_create_sql_function( ); return Err(ErrorCode::InvalidInputSyntax(format!( - "{}\n {}\n {}`{}`\n{}", + "{}\n{}\n{}`{}`\n{}", DEFAULT_ERR_MSG, invalid_msg, PROMPT, body, position )) .into()); From 0e42253d2d6d77d0f705bcb03088d2753cf45d63 Mon Sep 17 00:00:00 2001 From: Zihao Xu Date: Wed, 14 Feb 2024 11:22:26 +0800 Subject: [PATCH 11/25] add more test --- e2e_test/udf/sql_udf.slt | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/e2e_test/udf/sql_udf.slt b/e2e_test/udf/sql_udf.slt index a5780fb04f110..da5403fd9fce4 100644 --- a/e2e_test/udf/sql_udf.slt +++ b/e2e_test/udf/sql_udf.slt @@ -292,6 +292,32 @@ In SQL UDF definition: `select a + aa + a` ^^ +# With unnamed parameter +statement error +create function unnamed_param_hint(INT, INT) returns int language sql as 'select $1 + $3 + $2'; +---- +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 unnamed parameter $3 +In SQL UDF definition: `select $1 + $3 + $2` + ^^ + + +# A mixture of both +statement error +create function mix_hint(INT, aa INT, INT) returns int language sql as 'select $1 + aa + a + $2'; +---- +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 a +In SQL UDF definition: `select $1 + aa + a + $2` + ^ + + 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')'; From 974455a6f703918b68c001ab1591d1832331c027 Mon Sep 17 00:00:00 2001 From: Michael Xu Date: Mon, 19 Feb 2024 12:25:31 -0500 Subject: [PATCH 12/25] use word boundary instead of fragile manual lookahead --- src/frontend/src/handler/create_sql_function.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index 1fcdeb884b42b..b7d63b66fc5bb 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -62,11 +62,9 @@ fn create_mock_udf_context( /// Find the pattern for better hint display /// return the exact index where the pattern first appears fn find_target(input: &str, target: &str) -> Option { - // Regex pattern to find `target` not preceded/followed by an ASCII letter. - // \b asserts a word boundary, and \B asserts NOT a word boundary - // The pattern uses negative lookbehind (? Date: Mon, 19 Feb 2024 12:38:03 -0500 Subject: [PATCH 13/25] use SQL_UDF_PATTERN instead of manual encoding --- src/frontend/src/binder/expr/column.rs | 3 ++- src/frontend/src/binder/expr/mod.rs | 14 +++++++------- src/frontend/src/handler/create_sql_function.rs | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/frontend/src/binder/expr/column.rs b/src/frontend/src/binder/expr/column.rs index 369dcc4e2d2b3..38dcf11ba207f 100644 --- a/src/frontend/src/binder/expr/column.rs +++ b/src/frontend/src/binder/expr/column.rs @@ -18,6 +18,7 @@ use risingwave_sqlparser::ast::Ident; use crate::binder::Binder; use crate::error::{ErrorCode, Result}; use crate::expr::{CorrelatedInputRef, ExprImpl, ExprType, FunctionCall, InputRef, Literal}; +use crate::handler::create_sql_function::SQL_UDF_PATTERN; impl Binder { pub fn bind_column(&mut self, idents: &[Ident]) -> Result { @@ -57,7 +58,7 @@ impl Binder { // Note: the error message here also help with hint display // when invalid definition occurs at sql udf creation time return Err(ErrorCode::BindError(format!( - "[sql udf] failed to find named parameter {column_name}" + "{SQL_UDF_PATTERN} failed to find named parameter {column_name}" )) .into()); } diff --git a/src/frontend/src/binder/expr/mod.rs b/src/frontend/src/binder/expr/mod.rs index ebdc6c8679877..fd61cf205ac03 100644 --- a/src/frontend/src/binder/expr/mod.rs +++ b/src/frontend/src/binder/expr/mod.rs @@ -27,6 +27,7 @@ use crate::binder::expr::function::SYS_FUNCTION_WITHOUT_ARGS; use crate::binder::Binder; use crate::error::{ErrorCode, Result, RwError}; use crate::expr::{Expr as _, ExprImpl, ExprType, FunctionCall, InputRef, Parameter, SubqueryKind}; +use crate::handler::create_sql_function::SQL_UDF_PATTERN; mod binary_op; mod column; @@ -392,14 +393,13 @@ impl Binder { if self.udf_context.global_count() != 0 { if let Some(expr) = self.udf_context.get_expr(&format!("${index}")) { return Ok(expr.clone()); - } else { - // Same as `bind_column`, the error message here - // help with hint display when invalid definition occurs - return Err(ErrorCode::BindError(format!( - "[sql udf] failed to find unnamed parameter ${index}" - )) - .into()); } + // Same as `bind_column`, the error message here + // help with hint display when invalid definition occurs + return Err(ErrorCode::BindError(format!( + "{SQL_UDF_PATTERN} failed to find unnamed parameter ${index}" + )) + .into()); } Ok(Parameter::new(index, self.param_types.clone()).into()) diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index b7d63b66fc5bb..dc4338ae9e0cd 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -36,7 +36,7 @@ const DEFAULT_ERR_MSG: &str = "Failed to conduct semantic check"; const PROMPT: &str = "In SQL UDF definition: "; -const SQL_UDF_PATTERN: &str = "[sql udf]"; +pub const SQL_UDF_PATTERN: &str = "[sql udf]"; /// Create a mock `udf_context`, which is used for semantic check fn create_mock_udf_context( From dccf8fdef748068aab8afade9235403f75598dc9 Mon Sep 17 00:00:00 2001 From: Michael Xu Date: Mon, 19 Feb 2024 13:59:11 -0500 Subject: [PATCH 14/25] bring back lookahead technique when conducting regex pattern matching --- .../src/handler/create_sql_function.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index dc4338ae9e0cd..6de365f66599a 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -62,9 +62,10 @@ fn create_mock_udf_context( /// Find the pattern for better hint display /// return the exact index where the pattern first appears fn find_target(input: &str, target: &str) -> Option { - // Regex pattern to find `target` as a single word without any preceding or following characters - // here `\b` asserts a word boundary - let pattern = format!(r"\b{}\b", fancy_regex::escape(target)); + // Regex pattern to find `target` not preceded or followed by an ASCII letter + // The pattern uses negative lookbehind (? { + println!("e: {:#?}", e); if let ErrorCode::BindErrorRoot { expr: _, error } = e.inner() { let invalid_msg = error.to_string(); + println!("invalid_msg: {}", invalid_msg); + // Valid error message for hint display let Some(_) = invalid_msg.as_str().find(SQL_UDF_PATTERN) else { return Err( @@ -242,6 +246,8 @@ pub async fn handle_create_sql_function( let invalid_item_name = invalid_msg.split_whitespace().last().unwrap_or("null"); + println!("invalid_item_name: {}", invalid_item_name); + // Find the invalid parameter / column let Some(idx) = find_target(body.as_str(), invalid_item_name) else { return Err( @@ -261,10 +267,10 @@ pub async fn handle_create_sql_function( DEFAULT_ERR_MSG, invalid_msg, PROMPT, body, position )) .into()); - } else { - // Otherwise return the default error message - return Err(ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into()); } + + // Otherwise return the default error message + return Err(ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into()); } } } else { From 461fa4e0b85844abdccf2def389f4906ad08e097 Mon Sep 17 00:00:00 2001 From: Michael Xu Date: Mon, 19 Feb 2024 14:23:52 -0500 Subject: [PATCH 15/25] remove redundant debugging statement --- src/frontend/src/handler/create_sql_function.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index 6de365f66599a..435d1fe09f111 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -228,12 +228,9 @@ pub async fn handle_create_sql_function( } } Err(e) => { - println!("e: {:#?}", e); if let ErrorCode::BindErrorRoot { expr: _, error } = e.inner() { let invalid_msg = error.to_string(); - println!("invalid_msg: {}", invalid_msg); - // Valid error message for hint display let Some(_) = invalid_msg.as_str().find(SQL_UDF_PATTERN) else { return Err( @@ -246,8 +243,6 @@ pub async fn handle_create_sql_function( let invalid_item_name = invalid_msg.split_whitespace().last().unwrap_or("null"); - println!("invalid_item_name: {}", invalid_item_name); - // Find the invalid parameter / column let Some(idx) = find_target(body.as_str(), invalid_item_name) else { return Err( From 79e6bcc77b73019b520b0fda1b9b8abe0cc04130 Mon Sep 17 00:00:00 2001 From: Michael Xu Date: Mon, 19 Feb 2024 14:57:12 -0500 Subject: [PATCH 16/25] add more test cases --- e2e_test/udf/sql_udf.slt | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/e2e_test/udf/sql_udf.slt b/e2e_test/udf/sql_udf.slt index da5403fd9fce4..ed1a5ea327003 100644 --- a/e2e_test/udf/sql_udf.slt +++ b/e2e_test/udf/sql_udf.slt @@ -318,6 +318,20 @@ In SQL UDF definition: `select $1 + aa + a + $2` ^ +# Long invalid parameter +statement error +create function long_invalid_param(i_am_valid INT) returns int language sql as + 'select i_am_valid + _this_is_invalid_please_properly_mark_the_boundary_of_this_invalid_paramerter_ + i_am_valid'; +---- +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 _this_is_invalid_please_properly_mark_the_boundary_of_this_invalid_paramerter_ +In SQL UDF definition: `select i_am_valid + _this_is_invalid_please_properly_mark_the_boundary_of_this_invalid_paramerter_ + i_am_valid` + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + + 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')'; From e9e94b475264afde5bf3969f39332ca8894dfcaf Mon Sep 17 00:00:00 2001 From: Michael Xu Date: Tue, 20 Feb 2024 11:39:20 -0500 Subject: [PATCH 17/25] return None if regex compilation fails --- src/frontend/src/handler/create_sql_function.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index 435d1fe09f111..71fb480ae9ab3 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -66,7 +66,9 @@ fn find_target(input: &str, target: &str) -> Option { // The pattern uses negative lookbehind (? Date: Wed, 21 Feb 2024 15:53:41 -0500 Subject: [PATCH 18/25] add hint display for non-existent function --- .../src/handler/create_sql_function.rs | 102 +++++++++++++----- 1 file changed, 75 insertions(+), 27 deletions(-) diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index 71fb480ae9ab3..dd001082a3b17 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 item 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.find(SQL_UDF_PATTERN).is_some() { + ErrMsgType::Parameter + } else if invalid_msg.find(FUNCTION_KEYWORD).is_some() { + 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, @@ -232,20 +278,22 @@ pub async fn handle_create_sql_function( Err(e) => { if let ErrorCode::BindErrorRoot { expr: _, error } = e.inner() { let invalid_msg = error.to_string(); + + println!("invalid_msg: {}", invalid_msg); - // Valid error message for hint display - let Some(_) = invalid_msg.as_str().find(SQL_UDF_PATTERN) else { - return Err( - ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into() - ); - }; + // 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 invalid_item_name = - invalid_msg.split_whitespace().last().unwrap_or("null"); - - // Find the invalid parameter / column + 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() + ); + }; + + // 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() From 1005bc035c07f88f922bfaa92e5a52ed160f1729 Mon Sep 17 00:00:00 2001 From: Michael Xu Date: Wed, 21 Feb 2024 15:57:41 -0500 Subject: [PATCH 19/25] fix check --- src/frontend/src/handler/create_sql_function.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index dd001082a3b17..78d20caa82a92 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -278,7 +278,7 @@ pub async fn handle_create_sql_function( Err(e) => { if let ErrorCode::BindErrorRoot { expr: _, error } = e.inner() { let invalid_msg = error.to_string(); - + println!("invalid_msg: {}", invalid_msg); // First validate the message @@ -287,11 +287,12 @@ pub async fn handle_create_sql_function( // 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() - ); - }; + extract_hint_display_target(err_msg_type, invalid_msg.as_str()) + else { + return Err( + ErrorCode::InvalidInputSyntax(DEFAULT_ERR_MSG.into()).into() + ); + }; // Find the invalid parameter / column / function let Some(idx) = find_target(body.as_str(), invalid_item_name) else { From 96a0c3b4358a5492af735e4c5b398b57f377c21d Mon Sep 17 00:00:00 2001 From: Michael Xu Date: Wed, 21 Feb 2024 16:13:01 -0500 Subject: [PATCH 20/25] remove debug statement --- src/frontend/src/handler/create_sql_function.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index 78d20caa82a92..cb7c4b0907240 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -279,8 +279,6 @@ pub async fn handle_create_sql_function( if let ErrorCode::BindErrorRoot { expr: _, error } = e.inner() { let invalid_msg = error.to_string(); - println!("invalid_msg: {}", invalid_msg); - // First validate the message let err_msg_type = validate_err_msg(invalid_msg.as_str()); From a4cf35acdd4ab1adf0f69eaa2000af0d3fc66bed Mon Sep 17 00:00:00 2001 From: Michael Xu Date: Wed, 21 Feb 2024 16:19:54 -0500 Subject: [PATCH 21/25] fix check --- e2e_test/udf/sql_udf.slt | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/e2e_test/udf/sql_udf.slt b/e2e_test/udf/sql_udf.slt index ed1a5ea327003..9a87451f828c0 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)'; +---- +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 From ce30232d73e2507a210a009f993465989ca0be06 Mon Sep 17 00:00:00 2001 From: Michael Xu Date: Wed, 21 Feb 2024 16:36:24 -0500 Subject: [PATCH 22/25] update tests --- e2e_test/udf/sql_udf.slt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/e2e_test/udf/sql_udf.slt b/e2e_test/udf/sql_udf.slt index 9a87451f828c0..866f27abd52ce 100644 --- a/e2e_test/udf/sql_udf.slt +++ b/e2e_test/udf/sql_udf.slt @@ -336,7 +336,7 @@ In SQL UDF definition: `select i_am_valid + _this_is_invalid_please_properly_mar 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 +db error: ERROR: Failed to run the query Caused by: Invalid input syntax: Failed to conduct semantic check @@ -351,7 +351,7 @@ In SQL UDF definition: `select 1 + non_existent(1) + 1` 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 +db error: ERROR: Failed to run the query Caused by: Invalid input syntax: Failed to conduct semantic check @@ -367,7 +367,7 @@ create function call_regexp_replace() returns varchar language sql as 'select re statement error create function recursive(INT, INT) returns int language sql as 'select recursive($1, $2) + recursive($1, $2)'; ---- -ERROR: Failed to run the query +db error: ERROR: Failed to run the query Caused by: Invalid input syntax: Failed to conduct semantic check From bce64a625369118a6ee4b11f9d5452a5d56060db Mon Sep 17 00:00:00 2001 From: Zihao Xu Date: Wed, 21 Feb 2024 18:04:41 -0500 Subject: [PATCH 23/25] Update create_sql_function.rs --- src/frontend/src/handler/create_sql_function.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index cb7c4b0907240..cb5f3fd91e2bf 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -33,7 +33,7 @@ use crate::expr::{Expr, ExprImpl, Literal}; use crate::{bind_data_type, Binder}; /// The error type for hint display -/// Currently we will try invalid item first +/// Currently we will try invalid parameter first /// Then try to find non-existent functions enum ErrMsgType { Parameter, From 32314b49e901129c9cee0c2df8067ef2dde0deb8 Mon Sep 17 00:00:00 2001 From: Michael Xu Date: Wed, 21 Feb 2024 22:15:52 -0500 Subject: [PATCH 24/25] use contains instead of find --- src/frontend/src/handler/create_sql_function.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index cb5f3fd91e2bf..f0ab16a4be540 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -57,9 +57,9 @@ pub const SQL_UDF_PATTERN: &str = "[sql udf]"; /// it's possible to improve the display to users fn validate_err_msg(invalid_msg: &str) -> ErrMsgType { // First try invalid parameters - if invalid_msg.find(SQL_UDF_PATTERN).is_some() { + if invalid_msg.contains(SQL_UDF_PATTERN) { ErrMsgType::Parameter - } else if invalid_msg.find(FUNCTION_KEYWORD).is_some() { + } else if invalid_msg.contains(FUNCTION_KEYWORD) { ErrMsgType::Function } else { // Nothing could be better display From 85814d036669d6062483e5f41696bbbbab9e4ac8 Mon Sep 17 00:00:00 2001 From: Zihao Xu Date: Tue, 27 Feb 2024 06:27:32 +0800 Subject: [PATCH 25/25] fix check --- .../src/handler/create_sql_function.rs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/frontend/src/handler/create_sql_function.rs b/src/frontend/src/handler/create_sql_function.rs index 8fc9717f7b37b..f0ab16a4be540 100644 --- a/src/frontend/src/handler/create_sql_function.rs +++ b/src/frontend/src/handler/create_sql_function.rs @@ -123,24 +123,6 @@ fn create_mock_udf_context( ret } -/// Find the pattern for better hint display -/// return the exact index where the pattern first appears -fn find_target(input: &str, target: &str) -> Option { - // Regex pattern to find `target` not preceded or followed by an ASCII letter - // The pattern uses negative lookbehind (?