1fanwang opened a new pull request, #25970:
URL: https://github.com/apache/datafusion/pull/25970

   ## Which issue does this PR close?
   
   - Closes https://github.com/apache/datafusion/issues/25948.
   
   ## Rationale for this change
   
   array_concat on nested lists crashes or fails in cases that work with 
default inner fields. Concatenating a list whose inner lists have non-nullable 
elements with a one-dimensional array panics the process with "Arrays with 
inconsistent types passed to MutableArrayData". Nested lists whose inner list 
is a LargeList or has a non-default field name, and all-NULL arguments of 
different nesting depths, fail with an internal error saying array_concat 
returned a different type than the one promised at planning time.
   
   Planning built the result type from the leaf element type with default 
nullable inner fields, while execution kept the inner fields of the inputs, so 
the planned and executed types disagreed, and a non-nullable inner field met a 
nullable one in the same array builder.
   
   ## What changes are included in this PR?
   
   array_concat now plans its result from the argument types aligned to the 
deepest argument and unified with the same type union as before, so inner 
LargeList lists, field names and nullability carry through. Each argument is 
coerced to that type or to the matching lower-dimensional part of it, and 
execution builds the output, including an all-NULL result, with the planned 
type. Concatenating one-dimensional arrays is unchanged.
   
   ## What is the testing strategy for this PR?
   
   New cases in array/array_concat.slt cover each failure from the issue: a 
nested LargeList, an inner list with a non-default field name, all-NULL 
arguments of different depths, and non-nullable inner elements concatenated 
with a one-dimensional array.
   
   ### Testing Done
   
   The issue's queries in datafusion-cli, built with cargo build --profile ci 
-p datafusion-cli on main (416002a5b) and on this branch. The cargo registry 
path in the panic message is shortened.
   
   ```shell
   target/ci/datafusion-cli -f repro.sql; echo "exit=$?"
   ```
   
   On main, the first three queries fail with internal type errors and the 
fourth panics:
   
   ```text
   DataFusion CLI v55.1.0
   0 row(s) fetched. 
   Elapsed 0.025 seconds.
   
   Internal error: Assertion failed: result_data_type == *expected_type: 
Function 'array_concat' returned value of type 'List(LargeList(Int64))' while 
the following type was promised at planning time and expected: 
'List(List(Int64))'.
   This issue was likely caused by a bug in DataFusion's code. Please help us 
to resolve this by filing a bug report in our issue tracker: 
https://github.com/apache/datafusion/issues
   Internal error: Assertion failed: result_data_type == *expected_type: 
Function 'array_concat' returned value of type 'List(List(Int64, field: 
'element'))' while the following type was promised at planning time and 
expected: 'List(List(Int64))'.
   This issue was likely caused by a bug in DataFusion's code. Please help us 
to resolve this by filing a bug report in our issue tracker: 
https://github.com/apache/datafusion/issues
   Internal error: Assertion failed: result_data_type == *expected_type: 
Function 'array_concat' returned value of type 'List(Int64)' while the 
following type was promised at planning time and expected: 'List(List(Int64))'.
   This issue was likely caused by a bug in DataFusion's code. Please help us 
to resolve this by filing a bug report in our issue tracker: 
https://github.com/apache/datafusion/issues
   
   thread 'main' (35983318) panicked at 
<cargo-registry>/arrow-data-60.0.0/src/transform/mod.rs:464:13:
   assertion `left == right` failed: Arrays with inconsistent types passed to 
MutableArrayData
     left: List(Field { data_type: Int64 })
    right: List(Field { data_type: Int64, nullable: true })
   note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
   exit=101
   ```
   
   On this branch, all four queries return results:
   
   ```text
   DataFusion CLI v55.1.0
   0 row(s) fetched. 
   Elapsed 0.017 seconds.
   
   +-----------------------+
   | array_concat(t.a,t.a) |
   +-----------------------+
   | [[1, 2], [1, 2]]      |
   +-----------------------+
   1 row(s) fetched. 
   Elapsed 0.002 seconds.
   
   +-----------------------+
   | array_concat(t.b,t.b) |
   +-----------------------+
   | [[1], [1]]            |
   +-----------------------+
   1 row(s) fetched. 
   Elapsed 0.001 seconds.
   
   
+-----------------------------------------------------------------------------------------------+
   | 
array_concat(arrow_cast(NULL,Utf8("List(Int64)")),arrow_cast(NULL,Utf8("List(List(Int64))")))
 |
   
+-----------------------------------------------------------------------------------------------+
   | NULL                                                                       
                   |
   
+-----------------------------------------------------------------------------------------------+
   1 row(s) fetched. 
   Elapsed 0.001 seconds.
   
   +----------------------------------------+
   | array_concat(t.c,make_array(Int64(2))) |
   +----------------------------------------+
   | [[1], [2]]                             |
   +----------------------------------------+
   1 row(s) fetched. 
   Elapsed 0.001 seconds.
   
   exit=0
   ```
   
   <details>
   <summary>Reproducer source: repro.sql</summary>
   
   ```sql
   CREATE TABLE t AS SELECT
     make_array(arrow_cast([1, 2], 'LargeList(Int64)')) AS a,
     arrow_cast([[1]], 'List(List(Int64, field: ''element''), field: 
''element'')') AS b,
     arrow_cast(make_array(make_array(1)), 'List(List(non-null Int64))') AS c;
   
   SELECT array_concat(a, a) FROM t;
   SELECT array_concat(b, b) FROM t;
   SELECT array_concat(arrow_cast(NULL, 'List(Int64)'), arrow_cast(NULL, 
'List(List(Int64))'));
   SELECT array_concat(c, make_array(2)) FROM t;
   ```
   
   </details>
   
   ## Are there any user-facing changes?
   
   Yes. These array_concat calls now return results instead of panicking or 
failing with internal errors, and nested results keep the inner list types of 
their inputs. No API changes.
   


-- 
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]

Reply via email to