You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "1fanwang (via GitHub)" <gi...@apache.org> on 2026/10/02 11:11:44 UTC

[PR] fix: array_concat panics or fails on non-default nested lists [datafusion]

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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: array_concat panics or fails on non-default nested lists [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb merged PR #25970:
URL: https://github.com/apache/datafusion/pull/25970


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: array_concat panics or fails on non-default nested lists [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #25970:
URL: https://github.com/apache/datafusion/pull/25970#discussion_r4299224353


##########
datafusion/functions-nested/src/concat.rs:
##########
@@ -419,11 +476,8 @@ fn append_prepend_return_type(
 
 /// Concatenates the list arrays in `args` row-wise.
 ///
-/// `field` is the list field the output must carry. `array_concat` passes `None`
-/// because its `return_type` derives a fresh field from the unified element
-/// types, which is what deriving the field from the aligned inputs reproduces.
-/// `array_append` / `array_prepend` promise their input's field verbatim and so
-/// must pass it in explicitly.
+/// `field` is the list field the output must carry when it differs from the
+/// aligned inputs' first field.

Review Comment:
   Nit: `array_concat` now always passes `Some(field)` (the planned type), so this sentence is a bit vague. Maybe say who passes what:
   
   ```suggestion
   /// `field` is the list field the output must carry. `array_concat` passes the
   /// inner field of its planned return type, and `array_append` / `array_prepend`
   /// pass their input's field. `None` derives the field from the aligned inputs.
   ```



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: array_concat panics or fails on non-default nested lists [datafusion]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #25970:
URL: https://github.com/apache/datafusion/pull/25970#issuecomment-5951530026

   ## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/25970?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `85.00000%` with `12 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 82.61%. Comparing base ([`fca52aa`](https://app.codecov.io/gh/apache/datafusion/commit/fca52aa592e70899e33faf3074e0550a5d7f1256?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`c3fcddb`](https://app.codecov.io/gh/apache/datafusion/commit/c3fcddbc2d45be52d173a7faa7a5bff000375e1d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 1 commits behind head on main.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/25970?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [datafusion/functions-nested/src/concat.rs](https://app.codecov.io/gh/apache/datafusion/pull/25970?src=pr&el=tree&filepath=datafusion%2Ffunctions-nested%2Fsrc%2Fconcat.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9mdW5jdGlvbnMtbmVzdGVkL3NyYy9jb25jYXQucnM=) | 85.00% | [9 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/25970?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##             main   #25970      +/-   ##
   ==========================================
   + Coverage   82.59%   82.61%   +0.01%     
   ==========================================
     Files        1145     1145              
     Lines      444194   444357     +163     
     Branches   444194   444357     +163     
   ==========================================
   + Hits       366897   367101     +204     
   + Misses      55029    54963      -66     
   - Partials    22268    22293      +25     
   ```
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/25970?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: array_concat panics or fails on non-default nested lists [datafusion]

Posted by "1fanwang (via GitHub)" <gi...@apache.org>.
1fanwang commented on code in PR #25970:
URL: https://github.com/apache/datafusion/pull/25970#discussion_r4356643797


##########
datafusion/functions-nested/src/concat.rs:
##########
@@ -419,11 +476,8 @@ fn append_prepend_return_type(
 
 /// Concatenates the list arrays in `args` row-wise.
 ///
-/// `field` is the list field the output must carry. `array_concat` passes `None`
-/// because its `return_type` derives a fresh field from the unified element
-/// types, which is what deriving the field from the aligned inputs reproduces.
-/// `array_append` / `array_prepend` promise their input's field verbatim and so
-/// must pass it in explicitly.
+/// `field` is the list field the output must carry when it differs from the
+/// aligned inputs' first field.

Review Comment:
   Done in https://github.com/apache/datafusion/commit/5b91b532420990dc31d790fe2e301959be2b1f80. The doc now distinguishes the planned inner field used by array_concat from the input field used by array_append and array_prepend.



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: array_concat panics or fails on non-default nested lists [datafusion]

Posted by "1fanwang (via GitHub)" <gi...@apache.org>.
1fanwang commented on PR #25970:
URL: https://github.com/apache/datafusion/pull/25970#issuecomment-5957748659

   > If retaining this metadata is intentional, could you call it out in the description and add a small SLT case for identical named/non-nullable inputs?
   
   @Hasnaathussain it was not intentional. In https://github.com/apache/datafusion/commit/07cf61add4cc64feaf1a894597fc13f9c6956648 one-dimensional results go back to the default nullable item field, as on main, and array_concat.slt now pins List(Int64) for two inputs that share a named, non-nullable field, the case your probe caught. Nested results still keep their inner fields, which the issue's cases need. The description now shows the one-dimensional types on main and on this branch.
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org