'Rename `array()` function to `make_array()`, extend `array[]` #3122

alamb · 2022-08-13T11:57:03Z

~~Draft until~~:

builds on Minor: Clean up array test #3121
Needs improved type errors

Which issue does this PR close?

Closes #3115
Closes #3123

Rationale for this change

The core rationale is that this change is needed to support the next release of sqlparser-rs (which includes apache/datafusion-sqlparser-rs#566) which makes it impossible to use array as a function

More details here: apache/datafusion-sqlparser-rs#566 (comment)

The array() function is Spark syntax.

Postgres syntax is array[] and postgres actually treats the array keyword specially (it can not be used as a function)

alamb=# select array(1,2);
ERROR:  syntax error at or near "1"
LINE 1: select array(1,2);

What changes are included in this PR?

Rename array() to make_array() (so it doesn't conflict with postgres)
Update SQL planner to support array[] call make_array for array[] syntax
Require all arguments to make_array and array[] be the same type (as arrays have a single type for all elements)
Tests for same

Are there any user-facing changes?

array() is now written make_array()
array[] supports columns rather than just constants (e.g. array[column, column, ..] now works)
make_array() will error if the types are not exactly the same

alamb · 2022-08-15T22:02:10Z

datafusion/physical-expr/src/array_expressions.rs

+/// Create an array of FixedSizeList from a set of individual Arrays
+/// where each element in the output FixedSizeList is the result of
+/// concatenating the corresponding values in the input Arrays
+macro_rules! make_fixed_size_list {


I renamed this to be more descriptive of what it does (there are too may arrays in DataFusion already ;)

alamb · 2022-08-16T09:55:51Z

datafusion/core/tests/sql/functions.rs

-        "| [aaa, 3]                             |",
-        "+--------------------------------------+",
+        "+----------+",
+        "| array    |",


using array for the column name is consistent with postgres

alamb · 2022-08-16T09:56:24Z

datafusion/core/tests/sql/functions.rs

@@ -111,8 +111,8 @@ async fn query_concat() -> Result<()> {
    Ok(())
 }

-#[tokio::test]
-async fn query_array() -> Result<()> {
+// Return a session context with table "test" registered with 2 columns


The changes in this file illustrate the change in behavior that this PR makes

alamb · 2022-08-16T09:57:03Z

datafusion/core/tests/sql/functions.rs

+#[tokio::test]
+async fn query_array() {
+    let ctx = array_context();
+    let sql = "SELECT array[c1, cast(c2 as varchar)] FROM test";


Prior to this PR, this query would error (only constants were supported in array[] syntax. cc @ovr

alamb · 2022-08-16T09:57:43Z

datafusion/core/tests/sql/functions.rs

+async fn query_make_array_scalar() {
+    let ctx = SessionContext::new();
+
+    let sql = "SELECT make_array(1, 2, 3);";


This is the equivalent of SELECT array(1,2, 3) on master

alamb · 2022-08-16T09:58:08Z

datafusion/expr/src/function.rs

-            array_expressions::SUPPORTED_ARRAY_TYPES.to_vec(),
-            fun.volatility(),
-        ),
+        BuiltinScalarFunction::MakeArray => Signature::variadic_equal(fun.volatility()),


This requires all arguments to make_array be the same type

alamb · 2022-08-16T09:59:05Z

datafusion/physical-expr/src/array_expressions.rs

+/// Create an array of FixedSizeList from a set of individual Arrays
+/// where each element in the output FixedSizeList is the result of
+/// concatenating the corresponding values in the input Arrays
+macro_rules! make_fixed_size_list {


I simply renamed this macro to something that shows more of what it is doing

alamb · 2022-08-16T09:59:37Z

datafusion/sql/src/planner.rs

-        }
+        let fun = BuiltinScalarFunction::MakeArray;
+        // follow postgres convention and name result "array"
+        Ok(Expr::ScalarFunction { fun, args }.alias("array"))


This is the rewrite in the planner to write array[] syntax to a make_array function call

alamb · 2022-08-16T09:59:48Z

datafusion/sql/src/planner.rs

-        // HashSet doesn't guarantee order
-        assert_contains!(
-            err.to_string(),
-            r#"Arrays with different types are not supported: "#


this test is moved to function.rs

andygrove · 2022-08-18T19:59:13Z

@alamb I believe this is the next PR in line to be merged into sqlparser-0.21

andygrove · 2022-08-18T21:38:11Z

@alamb I believe this is the next PR in line to be merged into sqlparser-0.21

Never mind. These changes are not required in order to upgrade.

alamb · 2022-08-18T21:51:10Z

Update on this PR: I think I may have gotten too ambitious in this PR: array() somewhat confusingly returns a FixedSizedList and array[] returns a List.

Maybe we can just settle for updating the name of the array function to make_array 🤔 Will sleep on it

andygrove · 2022-08-18T22:10:34Z

Update on this PR: I think I may have gotten too ambitious in this PR: array() somewhat confusingly returns a FixedSizedList and array[] returns a List.

Maybe we can just settle for updating the name of the array function to make_array thinking Will sleep on it

I went ahead and created a PR just to do the rename: #3199

alamb · 2022-08-19T10:36:09Z

Thanks @andygrove -- I am going to abandon this PR for now. I learned a lot but I don't have time to figure out the implementation details of ListArrays more ;)

github-actions bot added core Core DataFusion crate logical-expr Logical plan and expressions physical-expr Physical Expressions sql SQL Planner labels Aug 13, 2022

alamb mentioned this pull request Aug 13, 2022

array function allows different data types #3123

Closed

alamb force-pushed the alamb/no_function_named_array branch from 5dd736e to a882e34 Compare August 15, 2022 22:34

This was referenced Aug 15, 2022

[Minor] Remove some redundant code #3169

Merged

array[] / make_array() will error if the types can not be coerced to a common type #3170

Open

alamb force-pushed the alamb/no_function_named_array branch from a882e34 to 65ab517 Compare August 15, 2022 22:42

alamb commented Aug 16, 2022

View reviewed changes

Rename array() function to make_array(), extend array[]

23845c8

alamb force-pushed the alamb/no_function_named_array branch from 65ab517 to 23845c8 Compare August 16, 2022 10:17

alamb mentioned this pull request Aug 18, 2022

sqlparser 0.21 upgrade #3192

Closed

5 tasks

alamb mentioned this pull request Aug 18, 2022

[Minor] Reduce code duplication creating ScalarValue::List #3197

Merged

andygrove mentioned this pull request Aug 18, 2022

[sqlparser-0.21] Rename function array to make_array #3199

Merged

alamb closed this Aug 19, 2022

izveigor mentioned this pull request Jun 6, 2023

Supports NULL in arrays #6556

Closed

alamb deleted the alamb/no_function_named_array branch August 8, 2023 20:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

'Rename `array()` function to `make_array()`, extend `array[]` #3122

'Rename `array()` function to `make_array()`, extend `array[]` #3122

alamb commented Aug 13, 2022 •

edited

Loading

alamb Aug 15, 2022

alamb Aug 16, 2022

alamb Aug 16, 2022

alamb Aug 16, 2022

ovr Aug 16, 2022

alamb Aug 16, 2022

alamb Aug 16, 2022

alamb Aug 16, 2022

alamb Aug 16, 2022

alamb Aug 16, 2022

andygrove commented Aug 18, 2022

andygrove commented Aug 18, 2022

alamb commented Aug 18, 2022

andygrove commented Aug 18, 2022

alamb commented Aug 19, 2022

'Rename array() function to make_array(), extend array[] #3122

'Rename array() function to make_array(), extend array[] #3122

Conversation

alamb commented Aug 13, 2022 • edited Loading

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andygrove commented Aug 18, 2022

andygrove commented Aug 18, 2022

alamb commented Aug 18, 2022

andygrove commented Aug 18, 2022

alamb commented Aug 19, 2022

'Rename `array()` function to `make_array()`, extend `array[]` #3122

'Rename `array()` function to `make_array()`, extend `array[]` #3122

alamb commented Aug 13, 2022 •

edited

Loading