From 95bf987d7651af24f61e06a7a77fce2b8f90bfea Mon Sep 17 00:00:00 2001 From: Weijun Huang Date: Sat, 19 Aug 2023 11:36:05 +0800 Subject: [PATCH 1/5] refine: substr error --- datafusion/expr/src/built_in_function.rs | 32 +++++++++---------- .../sqllogictest/test_files/functions.slt | 3 ++ 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/datafusion/expr/src/built_in_function.rs b/datafusion/expr/src/built_in_function.rs index 54ffc312a3bbf..2029c82035a0c 100644 --- a/datafusion/expr/src/built_in_function.rs +++ b/datafusion/expr/src/built_in_function.rs @@ -1395,25 +1395,25 @@ macro_rules! make_utf8_to_return_type { DataType::LargeUtf8 => $largeUtf8Type, DataType::Utf8 => $utf8Type, DataType::Null => DataType::Null, - DataType::Dictionary(_, value_type) => { - match **value_type { - DataType::LargeUtf8 => $largeUtf8Type, - DataType::Utf8 => $utf8Type, - DataType::Null => DataType::Null, - _ => { - // this error is internal as `data_types` should have captured this. - return internal_err!( - "The {:?} function can only accept strings.", - name - ); - } + DataType::Dictionary(_, value_type) => match **value_type { + DataType::LargeUtf8 => $largeUtf8Type, + DataType::Utf8 => $utf8Type, + DataType::Null => DataType::Null, + _ => { + // this error is internal as `data_types` should have captured this. + return internal_err!( + "The {:?} function can only accept strings, but got {:?}.", + name, + **value_type + ); } - } - _ => { + }, + data_type => { // this error is internal as `data_types` should have captured this. return internal_err!( - "The {:?} function can only accept strings.", - name + "The {:?} function can only accept strings, but got {:?}.", + name, + data_type ); } }) diff --git a/datafusion/sqllogictest/test_files/functions.slt b/datafusion/sqllogictest/test_files/functions.slt index f8dbf8a00d9d4..b1c213541c2b7 100644 --- a/datafusion/sqllogictest/test_files/functions.slt +++ b/datafusion/sqllogictest/test_files/functions.slt @@ -413,6 +413,9 @@ SELECT substr('alphabet', 3, CAST(NULL AS int)) ---- NULL +statement error The "substr" function can only accept strings, but got Int64. +SELECT substr(1, 3) + query T SELECT translate('12345', '143', 'ax') ---- From ddd5e634ecf74c3c7ba512a067f06ee498855a9b Mon Sep 17 00:00:00 2001 From: Weijun Huang Date: Sat, 19 Aug 2023 11:39:18 +0800 Subject: [PATCH 2/5] fix format --- datafusion/expr/src/built_in_function.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/datafusion/expr/src/built_in_function.rs b/datafusion/expr/src/built_in_function.rs index 2029c82035a0c..713dac57776dd 100644 --- a/datafusion/expr/src/built_in_function.rs +++ b/datafusion/expr/src/built_in_function.rs @@ -1400,12 +1400,12 @@ macro_rules! make_utf8_to_return_type { DataType::Utf8 => $utf8Type, DataType::Null => DataType::Null, _ => { - // this error is internal as `data_types` should have captured this. - return internal_err!( - "The {:?} function can only accept strings, but got {:?}.", - name, - **value_type - ); + // this error is internal as `data_types` should have captured this. + return internal_err!( + "The {:?} function can only accept strings, but got {:?}.", + name, + **value_type + ); } }, data_type => { From 8d7b90308d3638da4834e2b158c680000e3953fc Mon Sep 17 00:00:00 2001 From: Alex Huang Date: Sat, 19 Aug 2023 21:02:04 +0800 Subject: [PATCH 3/5] Update datafusion/expr/src/built_in_function.rs Co-authored-by: jakevin --- datafusion/expr/src/built_in_function.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datafusion/expr/src/built_in_function.rs b/datafusion/expr/src/built_in_function.rs index 713dac57776dd..4de8663e00a17 100644 --- a/datafusion/expr/src/built_in_function.rs +++ b/datafusion/expr/src/built_in_function.rs @@ -1410,7 +1410,7 @@ macro_rules! make_utf8_to_return_type { }, data_type => { // this error is internal as `data_types` should have captured this. - return internal_err!( + return plan_err!( "The {:?} function can only accept strings, but got {:?}.", name, data_type From 3a66359ab7d0390ead97c2e936580d9514a0a8e3 Mon Sep 17 00:00:00 2001 From: Alex Huang Date: Sat, 19 Aug 2023 21:02:11 +0800 Subject: [PATCH 4/5] Update datafusion/expr/src/built_in_function.rs Co-authored-by: jakevin --- datafusion/expr/src/built_in_function.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datafusion/expr/src/built_in_function.rs b/datafusion/expr/src/built_in_function.rs index 4de8663e00a17..e8b4654b97bd9 100644 --- a/datafusion/expr/src/built_in_function.rs +++ b/datafusion/expr/src/built_in_function.rs @@ -1401,7 +1401,7 @@ macro_rules! make_utf8_to_return_type { DataType::Null => DataType::Null, _ => { // this error is internal as `data_types` should have captured this. - return internal_err!( + return plan_err!( "The {:?} function can only accept strings, but got {:?}.", name, **value_type From e86c7186c9042d5c33f023578f8b7b7af98ef8e7 Mon Sep 17 00:00:00 2001 From: Weijun Huang Date: Sat, 19 Aug 2023 21:10:08 +0800 Subject: [PATCH 5/5] add more tests --- datafusion/sqllogictest/test_files/functions.slt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/datafusion/sqllogictest/test_files/functions.slt b/datafusion/sqllogictest/test_files/functions.slt index b1c213541c2b7..fd3a28dfe06d7 100644 --- a/datafusion/sqllogictest/test_files/functions.slt +++ b/datafusion/sqllogictest/test_files/functions.slt @@ -416,6 +416,9 @@ NULL statement error The "substr" function can only accept strings, but got Int64. SELECT substr(1, 3) +statement error The "substr" function can only accept strings, but got Int64. +SELECT substr(1, 3, 4) + query T SELECT translate('12345', '143', 'ax') ----