martin-g commented on code in PR #21047:
URL: https://github.com/apache/datafusion/pull/21047#discussion_r2959614007
##########
datafusion/functions-nested/src/arrays_zip.rs:
##########
@@ -49,30 +49,29 @@ struct ListColumnView {
make_udf_expr_and_func!(
ArraysZip,
arrays_zip,
- "combines multiple arrays into a single array of structs.",
+ "combines one or multiple arrays into a single array of structs.",
arrays_zip_udf
);
#[user_doc(
doc_section(label = "Array Functions"),
description = "Returns an array of structs created by combining the
elements of each input array at the same index. If the arrays have different
lengths, shorter arrays are padded with NULLs.",
- syntax_example = "arrays_zip(array1, array2[, ..., array_n])",
+ syntax_example = "arrays_zip(array1[, ..., array_n])",
sql_example = r#"```sql
> select arrays_zip([1, 2, 3], ['a', 'b', 'c']);
+---------------------------------------------------+
| arrays_zip([1, 2, 3], ['a', 'b', 'c']) |
+---------------------------------------------------+
-| [{c0: 1, c1: a}, {c0: 2, c1: b}, {c0: 3, c1: c}] |
+| [{1: 1, 2: a}, {1: 2, 2: b}, {1: 3, 2: c}] |
+---------------------------------------------------+
> select arrays_zip([1, 2], [3, 4, 5]);
Review Comment:
There are two examples both with 2 arrays. Does it make sense to add an
example with 1 array too ? Or maybe edit one of the existing examples
##########
datafusion/functions-nested/src/arrays_zip.rs:
##########
@@ -49,30 +49,29 @@ struct ListColumnView {
make_udf_expr_and_func!(
ArraysZip,
arrays_zip,
- "combines multiple arrays into a single array of structs.",
+ "combines one or multiple arrays into a single array of structs.",
arrays_zip_udf
);
#[user_doc(
doc_section(label = "Array Functions"),
description = "Returns an array of structs created by combining the
elements of each input array at the same index. If the arrays have different
lengths, shorter arrays are padded with NULLs.",
- syntax_example = "arrays_zip(array1, array2[, ..., array_n])",
+ syntax_example = "arrays_zip(array1[, ..., array_n])",
sql_example = r#"```sql
> select arrays_zip([1, 2, 3], ['a', 'b', 'c']);
+---------------------------------------------------+
| arrays_zip([1, 2, 3], ['a', 'b', 'c']) |
+---------------------------------------------------+
-| [{c0: 1, c1: a}, {c0: 2, c1: b}, {c0: 3, c1: c}] |
+| [{1: 1, 2: a}, {1: 2, 2: b}, {1: 3, 2: c}] |
+---------------------------------------------------+
> select arrays_zip([1, 2], [3, 4, 5]);
+---------------------------------------------------+
| arrays_zip([1, 2], [3, 4, 5]) |
+---------------------------------------------------+
-| [{c0: 1, c1: 3}, {c0: 2, c1: 4}, {c0: , c1: 5}] |
+| [{1: 1, 2: 3}, {1: 2, 2: 4}, {1: , 2: 5}] |
Review Comment:
```suggestion
| [{1: 1, 2: 3}, {1: 2, 2: 4}, {1: NULL, 2: 5}] |
```
##########
datafusion/sqllogictest/test_files/array.slt:
##########
@@ -7377,6 +7429,12 @@ select arrays_zip(
----
[{1: 1, 2: 10}, {1: 2, 2: 20}, {1: 3, 2: 30}]
+# single argument from FixedSizeList
+query ?
+select arrays_zip(arrow_cast(make_array(1, 2, 3), 'FixedSizeList(3, Int64)'));
+----
+[{1: 1}, {1: 2}, {1: 3}]
+
# arrays_zip mixing List and LargeList
Review Comment:
Please also add a test for `select arrays_zip();`, i.e. to test that it
needs at least one argument
##########
datafusion/functions-nested/src/arrays_zip.rs:
##########
@@ -49,30 +49,29 @@ struct ListColumnView {
make_udf_expr_and_func!(
ArraysZip,
arrays_zip,
- "combines multiple arrays into a single array of structs.",
+ "combines one or multiple arrays into a single array of structs.",
arrays_zip_udf
);
#[user_doc(
doc_section(label = "Array Functions"),
description = "Returns an array of structs created by combining the
elements of each input array at the same index. If the arrays have different
lengths, shorter arrays are padded with NULLs.",
- syntax_example = "arrays_zip(array1, array2[, ..., array_n])",
+ syntax_example = "arrays_zip(array1[, ..., array_n])",
sql_example = r#"```sql
> select arrays_zip([1, 2, 3], ['a', 'b', 'c']);
+---------------------------------------------------+
| arrays_zip([1, 2, 3], ['a', 'b', 'c']) |
+---------------------------------------------------+
-| [{c0: 1, c1: a}, {c0: 2, c1: b}, {c0: 3, c1: c}] |
+| [{1: 1, 2: a}, {1: 2, 2: b}, {1: 3, 2: c}] |
+---------------------------------------------------+
> select arrays_zip([1, 2], [3, 4, 5]);
+---------------------------------------------------+
| arrays_zip([1, 2], [3, 4, 5]) |
+---------------------------------------------------+
-| [{c0: 1, c1: 3}, {c0: 2, c1: 4}, {c0: , c1: 5}] |
+| [{1: 1, 2: 3}, {1: 2, 2: 4}, {1: , 2: 5}] |
+---------------------------------------------------+
```"#,
argument(name = "array1", description = "First array expression."),
- argument(name = "array2", description = "Second array expression."),
argument(name = "array_n", description = "Subsequent array expressions.")
Review Comment:
```suggestion
argument(name = "array_n", description = "Optional additional array
expressions.")
```
##########
datafusion/sqllogictest/test_files/array.slt:
##########
@@ -7377,6 +7429,12 @@ select arrays_zip(
----
[{1: 1, 2: 10}, {1: 2, 2: 20}, {1: 3, 2: 30}]
+# single argument from FixedSizeList
+query ?
+select arrays_zip(arrow_cast(make_array(1, 2, 3), 'FixedSizeList(3, Int64)'));
+----
+[{1: 1}, {1: 2}, {1: 3}]
+
# arrays_zip mixing List and LargeList
Review Comment:
Also I don't see a test for something like:
```
select arrays_zip(1, 2);
Execution error: arrays_zip expects array arguments, got Int64
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
