This is an automated email from the ASF dual-hosted git repository.

alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git


The following commit(s) were added to refs/heads/main by this push:
     new bfe3e4255e Change error type of invalid argument to PlanError rather 
than InternalError, remove misleading comments (#7355)
bfe3e4255e is described below

commit bfe3e4255e4a233c9c17aabd512e144336e618fc
Author: Andrew Lamb <[email protected]>
AuthorDate: Wed Aug 23 12:35:16 2023 -0400

    Change error type of invalid argument to PlanError rather than 
InternalError, remove misleading comments (#7355)
    
    * Fix error type of invalid argument types, remove misleading comments
    
    * tests for arrays
    
    * more tests
    
    * Add more tests
    
    * fix error
---
 datafusion/expr/src/built_in_function.rs          | 25 ++++++++---------------
 datafusion/sqllogictest/test_files/array.slt      | 14 +++++++++++++
 datafusion/sqllogictest/test_files/encoding.slt   | 25 +++++++++++++++++++++++
 datafusion/sqllogictest/test_files/predicates.slt |  4 ++++
 datafusion/sqllogictest/test_files/timestamps.slt |  4 ++++
 5 files changed, 55 insertions(+), 17 deletions(-)

diff --git a/datafusion/expr/src/built_in_function.rs 
b/datafusion/expr/src/built_in_function.rs
index 9a4eb74c53..e97269e474 100644
--- a/datafusion/expr/src/built_in_function.rs
+++ b/datafusion/expr/src/built_in_function.rs
@@ -528,7 +528,7 @@ impl BuiltinScalarFunction {
                             }
                         }
                         _ => {
-                            return internal_err!(
+                            return plan_err!(
                                 "The {self} function can only accept list as 
the args."
                             )
                         }
@@ -546,7 +546,7 @@ impl BuiltinScalarFunction {
             }
             BuiltinScalarFunction::ArrayElement => match &input_expr_types[0] {
                 List(field) => Ok(field.data_type().clone()),
-                _ => internal_err!(
+                _ => plan_err!(
                     "The {self} function can only accept list as the first 
argument"
                 ),
             },
@@ -612,7 +612,7 @@ impl BuiltinScalarFunction {
                     Timestamp(Microsecond, _) => Ok(Timestamp(Microsecond, 
None)),
                     Timestamp(Millisecond, _) => Ok(Timestamp(Millisecond, 
None)),
                     Timestamp(Second, _) => Ok(Timestamp(Second, None)),
-                    _ => internal_err!(
+                    _ => plan_err!(
                     "The {self} function can only accept timestamp as the 
second arg."
                 ),
                 }
@@ -681,8 +681,7 @@ impl BuiltinScalarFunction {
                 LargeBinary => LargeUtf8,
                 Null => Null,
                 _ => {
-                    // this error is internal as `data_types` should have 
captured this.
-                    return internal_err!(
+                    return plan_err!(
                         "The encode function can only accept utf8 or binary."
                     );
                 }
@@ -694,8 +693,7 @@ impl BuiltinScalarFunction {
                 LargeBinary => LargeBinary,
                 Null => Null,
                 _ => {
-                    // this error is internal as `data_types` should have 
captured this.
-                    return internal_err!(
+                    return plan_err!(
                         "The decode function can only accept utf8 or binary."
                     );
                 }
@@ -713,10 +711,7 @@ impl BuiltinScalarFunction {
             BuiltinScalarFunction::ToHex => Ok(match input_expr_types[0] {
                 Int8 | Int16 | Int32 | Int64 => Utf8,
                 _ => {
-                    // this error is internal as `data_types` should have 
captured this.
-                    return internal_err!(
-                        "The to_hex function can only accept integers."
-                    );
+                    return plan_err!("The to_hex function can only accept 
integers.");
                 }
             }),
             BuiltinScalarFunction::ToTimestamp => Ok(Timestamp(Nanosecond, 
None)),
@@ -741,8 +736,7 @@ impl BuiltinScalarFunction {
                 Utf8 => List(Arc::new(Field::new("item", Utf8, true))),
                 Null => Null,
                 _ => {
-                    // this error is internal as `data_types` should have 
captured this.
-                    return internal_err!(
+                    return plan_err!(
                         "The regexp_extract function can only accept strings."
                     );
                 }
@@ -1406,7 +1400,6 @@ 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 plan_err!(
                             "The {:?} function can only accept strings, but 
got {:?}.",
                             name,
@@ -1415,7 +1408,6 @@ macro_rules! make_utf8_to_return_type {
                     }
                 },
                 data_type => {
-                    // this error is internal as `data_types` should have 
captured this.
                     return plan_err!(
                         "The {:?} function can only accept strings, but got 
{:?}.",
                         name,
@@ -1438,8 +1430,7 @@ fn utf8_or_binary_to_binary_type(arg_type: &DataType, 
name: &str) -> Result<Data
         | DataType::LargeBinary => DataType::Binary,
         DataType::Null => DataType::Null,
         _ => {
-            // this error is internal as `data_types` should have captured 
this.
-            return internal_err!(
+            return plan_err!(
                 "The {name:?} function can only accept strings or binary 
arrays."
             );
         }
diff --git a/datafusion/sqllogictest/test_files/array.slt 
b/datafusion/sqllogictest/test_files/array.slt
index bd16072b29..a4969b1e20 100644
--- a/datafusion/sqllogictest/test_files/array.slt
+++ b/datafusion/sqllogictest/test_files/array.slt
@@ -600,6 +600,11 @@ from arrays_values_without_nulls;
 
 ## array_element (aliases: array_extract, list_extract, list_element)
 
+# array_element error
+query error DataFusion error: Error during planning: The array_element 
function can only accept list as the first argument
+select array_element(1, 2);
+
+
 # array_element scalar function #1 (with positive index)
 query IT
 select array_element(make_array(1, 2, 3, 4, 5), 2), 
array_element(make_array('h', 'e', 'l', 'l', 'o'), 3);
@@ -1098,6 +1103,10 @@ select array_repeat([1], column3), array_repeat(column1, 
3) from arrays_values_w
 
 ## array_concat (aliases: `array_cat`, `list_concat`, `list_cat`)
 
+# array_concat error
+query error DataFusion error: Error during planning: The array_concat function 
can only accept list as the args\.
+select array_concat(1, 2);
+
 # array_concat scalar function #1
 query ??
 select array_concat(make_array(1, 2, 3), make_array(4, 5, 6), make_array(7, 8, 
9)), array_concat(make_array([1], [2]), make_array([3], [4]));
@@ -2008,6 +2017,11 @@ NULL 10
 
 ## array_dims (aliases: `list_dims`)
 
+# array dims error
+# TODO this is a separate bug
+query error Internal error: could not cast value to 
arrow_array::array::list_array::GenericListArray<i32>\.
+select array_dims(1);
+
 # array_dims scalar function
 query ???
 select array_dims(make_array(1, 2, 3)), array_dims(make_array([1, 2], [3, 
4])), array_dims(make_array([[[[1], [2]]]]));
diff --git a/datafusion/sqllogictest/test_files/encoding.slt 
b/datafusion/sqllogictest/test_files/encoding.slt
index b16ceebd6d..dcbf238b5f 100644
--- a/datafusion/sqllogictest/test_files/encoding.slt
+++ b/datafusion/sqllogictest/test_files/encoding.slt
@@ -27,6 +27,24 @@ CREATE TABLE test(
   (2, NULL, NULL, NULL)
 ;
 
+# errors
+query error DataFusion error: Error during planning: The encode function can 
only accept utf8 or binary\.
+select encode(12, 'hex')
+
+query error DataFusion error: Error during planning: There is no built\-in 
encoding named 'non_encoding', currently supported encodings are: base64, hex
+select encode(bin_field, 'non_encoding') from test;
+
+query error DataFusion error: Error during planning: The decode function can 
only accept utf8 or binary\.
+select decode(12, 'hex')
+
+query error DataFusion error: Error during planning: There is no built\-in 
encoding named 'non_encoding', currently supported encodings are: base64, hex
+select decode(hex_field, 'non_encoding') from test;
+
+query error DataFusion error: SQL error: ParserError\("Expected an SQL 
statement, found: Candidate"\)
+       Candidate functions:
+       to_hex\(Int64\)
+select to_hex(hex_field) from test;
+
 # Arrays tests
 query T
 SELECT encode(bin_field, 'hex') FROM test ORDER BY num;
@@ -48,3 +66,10 @@ SELECT arrow_cast(decode(hex_field, 'hex'), 'Utf8') FROM 
test ORDER BY num;
 abc
 qweqwe
 NULL
+
+query T
+select to_hex(num) from test ORDER BY num;
+----
+0
+1
+2
diff --git a/datafusion/sqllogictest/test_files/predicates.slt 
b/datafusion/sqllogictest/test_files/predicates.slt
index 2bd20b8106..aa233115b5 100644
--- a/datafusion/sqllogictest/test_files/predicates.slt
+++ b/datafusion/sqllogictest/test_files/predicates.slt
@@ -192,6 +192,10 @@ statement ok
 CREATE TABLE IF NOT EXISTS test AS VALUES('foo'),('Barrr'),('Bazzz'),('ZZZZZ');
 
 # async fn test_regexp_is_match
+query error Error during planning: Cannot infer common argument type for regex 
operation Int64 \~ Utf8
+SELECT * FROM test WHERE 12 ~ 'z'
+
+
 query T
 SELECT * FROM test WHERE column1 ~ 'z'
 ----
diff --git a/datafusion/sqllogictest/test_files/timestamps.slt 
b/datafusion/sqllogictest/test_files/timestamps.slt
index 72f168e014..abb74b468e 100644
--- a/datafusion/sqllogictest/test_files/timestamps.slt
+++ b/datafusion/sqllogictest/test_files/timestamps.slt
@@ -390,6 +390,10 @@ select to_timestamp_seconds(cast (1 as int));
 ## test date_bin function
 ##########
 
+# invalid second arg type
+query error DataFusion error: Error during planning: No function matches the 
given name and argument types 'date_bin\(Interval\(MonthDayNano\), Int64, 
Timestamp\(Nanosecond, None\)\)'\.
+SELECT DATE_BIN(INTERVAL '0 second', 25, TIMESTAMP '1970-01-01T00:00:00Z')
+
 # not support interval 0
 statement error Execution error: DATE_BIN stride must be non-zero
 SELECT DATE_BIN(INTERVAL '0 second', TIMESTAMP '2022-08-03 
14:38:50.000000006Z', TIMESTAMP '1970-01-01T00:00:00Z')

Reply via email to