From bd9f0cfc6f6571e9b495c1dbce481c4693f44964 Mon Sep 17 00:00:00 2001 From: jonahgao Date: Fri, 26 Apr 2024 11:02:01 +0800 Subject: [PATCH 1/5] fix: no longer support the `substring` function --- datafusion/sql/src/expr/mod.rs | 2 +- .../sqllogictest/test_files/functions.slt | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs index 0d1db8a29cce..5bf485591d08 100644 --- a/datafusion/sql/src/expr/mod.rs +++ b/datafusion/sql/src/expr/mod.rs @@ -467,7 +467,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { expr, substring_from, substring_for, - special: false, + special: true, } => self.sql_substring_to_expr( expr, substring_from, diff --git a/datafusion/sqllogictest/test_files/functions.slt b/datafusion/sqllogictest/test_files/functions.slt index d0d2bac59e91..3c06d6c37625 100644 --- a/datafusion/sqllogictest/test_files/functions.slt +++ b/datafusion/sqllogictest/test_files/functions.slt @@ -419,6 +419,26 @@ SELECT substr(1, 3) statement error The SUBSTR function can only accept strings, but got Int64. SELECT substr(1, 3, 4) +query T +SELECT substring('alphabet', 1) +--- +---- +alphabet + +query T +SELECT substring('alphabet', 3, 2) +---- +ph + +query error DataFusion error: This feature is not implemented: Unsupported ast node in sqltorel: Substring \{ expr: Value\(SingleQuotedString\("alphabet"\)\), substring_from: Some\(Value\(Number\("3", false\)\)\), substring_for: None, special: false \} +SELECT substring('alphabet' FROM 3) + +query error DataFusion error: This feature is not implemented: Unsupported ast node in sqltorel: Substring \{ expr: Value\(SingleQuotedString\("alphabet"\)\), substring_from: Some\(Value\(Number\("3", false\)\)\), substring_for: Some\(Value\(Number\("2", false\)\)\), special: false \} +SELECT substring('alphabet' FROM 3 FOR 2) + +query error DataFusion error: This feature is not implemented: Unsupported ast node in sqltorel: Substring \{ expr: Value\(SingleQuotedString\("alphabet"\)\), substring_from: None, substring_for: Some\(Value\(Number\("2", false\)\)\), special: false \} +SELECT substring('alphabet' FOR 2) + query T SELECT translate('12345', '143', 'ax') ---- From a91df0ebfe893078382d864a6297c7bdc527f3db Mon Sep 17 00:00:00 2001 From: jonahgao Date: Fri, 26 Apr 2024 11:28:01 +0800 Subject: [PATCH 2/5] enable from-for format --- datafusion/sql/src/expr/mod.rs | 9 +----- datafusion/sqllogictest/test_files/expr.slt | 28 +++++++++++++++++++ .../sqllogictest/test_files/functions.slt | 20 ------------- 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs index 5bf485591d08..ee4ae4657923 100644 --- a/datafusion/sql/src/expr/mod.rs +++ b/datafusion/sql/src/expr/mod.rs @@ -467,7 +467,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { expr, substring_from, substring_for, - special: true, + .. } => self.sql_substring_to_expr( expr, substring_from, @@ -476,13 +476,6 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { planner_context, ), - #[cfg(not(feature = "unicode_expressions"))] - SQLExpr::Substring { .. } => { - internal_err!( - "statement substring requires compilation with feature flag: unicode_expressions." - ) - } - SQLExpr::Trim { expr, trim_where, diff --git a/datafusion/sqllogictest/test_files/expr.slt b/datafusion/sqllogictest/test_files/expr.slt index adc577f12f91..06928393fdc9 100644 --- a/datafusion/sqllogictest/test_files/expr.slt +++ b/datafusion/sqllogictest/test_files/expr.slt @@ -1871,6 +1871,17 @@ SELECT digest('','blake3'); ---- af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262 + +query T +SELECT substring('alphabet', 1) +---- +alphabet + +query T +SELECT substring('alphabet', 3, 2) +---- +ph + query T SELECT substring('alphabet' from 2 for 1); ---- @@ -1886,6 +1897,23 @@ SELECT substring('alphabet' for 1); ---- a +# String arguments are not supported because they might contain regular expressions. +query error DataFusion error: Error during planning: No function matches the given name and argument types +SELECT substring('alphabet' FROM '3') + +query error DataFusion error: Error during planning: No function matches the given name and argument types +SELECT substring('alphabet' FROM '3' FOR '2') + +query error DataFusion error: Error during planning: No function matches the given name and argument types +SELECT substring('alphabet' FROM '3' FOR 2) + +query error DataFusion error: Error during planning: No function matches the given name and argument types +SELECT substring('alphabet' FROM 3 FOR '2') + +query error DataFusion error: Error during planning: No function matches the given name and argument types +SELECT substring('alphabet' FOR '2') + + ##### csv_query_nullif_divide_by_0 diff --git a/datafusion/sqllogictest/test_files/functions.slt b/datafusion/sqllogictest/test_files/functions.slt index 3c06d6c37625..d0d2bac59e91 100644 --- a/datafusion/sqllogictest/test_files/functions.slt +++ b/datafusion/sqllogictest/test_files/functions.slt @@ -419,26 +419,6 @@ SELECT substr(1, 3) statement error The SUBSTR function can only accept strings, but got Int64. SELECT substr(1, 3, 4) -query T -SELECT substring('alphabet', 1) ---- ----- -alphabet - -query T -SELECT substring('alphabet', 3, 2) ----- -ph - -query error DataFusion error: This feature is not implemented: Unsupported ast node in sqltorel: Substring \{ expr: Value\(SingleQuotedString\("alphabet"\)\), substring_from: Some\(Value\(Number\("3", false\)\)\), substring_for: None, special: false \} -SELECT substring('alphabet' FROM 3) - -query error DataFusion error: This feature is not implemented: Unsupported ast node in sqltorel: Substring \{ expr: Value\(SingleQuotedString\("alphabet"\)\), substring_from: Some\(Value\(Number\("3", false\)\)\), substring_for: Some\(Value\(Number\("2", false\)\)\), special: false \} -SELECT substring('alphabet' FROM 3 FOR 2) - -query error DataFusion error: This feature is not implemented: Unsupported ast node in sqltorel: Substring \{ expr: Value\(SingleQuotedString\("alphabet"\)\), substring_from: None, substring_for: Some\(Value\(Number\("2", false\)\)\), special: false \} -SELECT substring('alphabet' FOR 2) - query T SELECT translate('12345', '143', 'ax') ---- From b48be41aa95a3cbd9e16dc9daa4bca8c0badb788 Mon Sep 17 00:00:00 2001 From: jonahgao Date: Fri, 26 Apr 2024 13:14:18 +0800 Subject: [PATCH 3/5] update test comment --- datafusion/sqllogictest/test_files/expr.slt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/datafusion/sqllogictest/test_files/expr.slt b/datafusion/sqllogictest/test_files/expr.slt index 06928393fdc9..ff63416b3a10 100644 --- a/datafusion/sqllogictest/test_files/expr.slt +++ b/datafusion/sqllogictest/test_files/expr.slt @@ -1897,7 +1897,8 @@ SELECT substring('alphabet' for 1); ---- a -# String arguments are not supported because they might contain regular expressions. +# The 'from' and 'for' parameters don't support string types, because they should be treated as +# regular expressions, which we have not implemented yet. query error DataFusion error: Error during planning: No function matches the given name and argument types SELECT substring('alphabet' FROM '3') From 0e9253e15c1a59ca5b164720afd70189f4689c2f Mon Sep 17 00:00:00 2001 From: jonahgao Date: Fri, 26 Apr 2024 20:47:58 +0800 Subject: [PATCH 4/5] review feedback --- datafusion/sql/src/expr/mod.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs index ee4ae4657923..13f559a0ebc7 100644 --- a/datafusion/sql/src/expr/mod.rs +++ b/datafusion/sql/src/expr/mod.rs @@ -467,7 +467,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { expr, substring_from, substring_for, - .. + special: _, } => self.sql_substring_to_expr( expr, substring_from, @@ -476,6 +476,13 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { planner_context, ), + #[cfg(not(feature = "unicode_expressions"))] + SQLExpr::Substring { .. } => { + internal_err!( + "statement substring requires compilation with feature flag: unicode_expressions." + ) + } + SQLExpr::Trim { expr, trim_where, From 0d0fbe891cae0eac54407c1aa6930902b68a6575 Mon Sep 17 00:00:00 2001 From: jonahgao Date: Fri, 26 Apr 2024 20:47:58 +0800 Subject: [PATCH 5/5] review feedback Co-authored-by: Jeffrey Vo --- datafusion/sql/src/expr/mod.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/datafusion/sql/src/expr/mod.rs b/datafusion/sql/src/expr/mod.rs index ee4ae4657923..13f559a0ebc7 100644 --- a/datafusion/sql/src/expr/mod.rs +++ b/datafusion/sql/src/expr/mod.rs @@ -467,7 +467,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { expr, substring_from, substring_for, - .. + special: _, } => self.sql_substring_to_expr( expr, substring_from, @@ -476,6 +476,13 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { planner_context, ), + #[cfg(not(feature = "unicode_expressions"))] + SQLExpr::Substring { .. } => { + internal_err!( + "statement substring requires compilation with feature flag: unicode_expressions." + ) + } + SQLExpr::Trim { expr, trim_where,