[branch-55] fix: preserve the input list's inner field in array_append/prepend/replace - #24365 - #24377
Merged
alamb merged 1 commit intoAug 14, 2026
Conversation
…place (apache#24365) ## Which issue does this PR close? - Closes apache#24347. ## Rationale for this change `array_append`, `array_prepend`, `array_replace`, `array_replace_n` and `array_replace_all` promise the input list type verbatim — inner field name, nullability and metadata included — but their kernels rebuilt the output's inner field from scratch with `Field::new_list_field(..., true)`. On debug builds this trips the return-type assertion from apache#17515; on release builds it silently yields a batch whose inner field disagrees with the schema the planner recorded. This is the other half of apache#24341, whose `array_slice` part was fixed in apache#24345. The field-name symptom is a 55.0.0 regression from the same commit (5b22857, apache#20945); the non-nullable symptom is not a regression. Unlike `array_slice`, threading the input's field through is not sufficient here: the appended, prepended or replacement element can itself be null, so a promise cloned from a `List(non-null T)` input is wrong at the source and arrow rejects the array with `Non-nullable field of ListArray cannot contain nulls`. ## What changes are included in this PR? Each of the five functions now implements `return_field_from_args`, carrying the input field's name and metadata through while widening `nullable` when the new element's argument is nullable. The kernels build their output from `args.return_field` instead of deriving a field of their own, so promise and payload come from a single source. Nullability is therefore only widened when the result can genuinely contain a null: ```sql array_append(List(non-null Int64), 3) -> List(non-null Int64) array_append(List(non-null Int64), NULL) -> List(Int64) ``` Two paths beyond those listed in the issue turned out to have the same defect and are fixed too: `array_append` / `array_prepend` with a **nested** value type (which delegates to `concat_internal`), and the `LargeList` variants of all five. Since these five now implement `return_field_from_args`, their `return_type` becomes unreachable and returns `internal_err!("return_field_from_args should be used instead")`, matching the guidance on `ScalarUDFImpl::return_type` and the existing convention in `remove.rs` and `map_values.rs`. Two small cleanups while in here: - The `List`/`LargeList` inner-field extraction added to `general_array_slice` by apache#24345 is now shared as `utils::list_inner_field`, used by all three files. Its error text is unchanged. The `ListView` variant in `general_list_view_array_slice` is deliberately left alone — folding all four variants into one helper would let `general_array_slice` silently accept a `ListView` that its match currently rejects. - `array_concat` is **not** affected and its behaviour is unchanged: it derives a fresh return type via `type_union_resolution` rather than cloning an input's, so it keeps passing `None` to `concat_internal` and deriving the field from the aligned inputs. Behaviour for `Null`-typed array arguments is unchanged in all five functions (`array_replace*` return `Null`, `array_append` / `array_prepend` return `List(element)`). ## Are these changes tested? Yes — 14 new SLT tests across `array_append.slt`, `array_prepend.slt`, `array_replace.slt`, plus two guard cases in `array_concat.slt` pinning down that it is unaffected. `arrow_cast` can express a named inner field (`'List(Int64, field: ''element'')'`), so these reproduce the field-name half of the bug without needing the Spark dialect. Coverage: inner field name preserved, non-nullable inner field preserved, nullability widened only when the new element is nullable (both literal `NULL` and a nullable column), `LargeList`, nested value types, the `max <= 0` short circuit, and a `NULL` `max`. Every one of these queries fails on `main` with the return-type assertion. Also run: `cargo clippy --all-targets --all-features -- -D warnings`, the full sqllogictest suite, and the extended workspace test suite (68 test binaries, 0 failures). The `array_replace` and `array_concat` benchmarks show no regression against `main`. ## Are there any user-facing changes? The five functions now return the inner field they promise instead of a rebuilt one, which is the bug fix. As a consequence, appending or replacing with a nullable element widens the declared inner nullability of the result (`List(non-null Int64)` -> `List(Int64)`), which is required for the result to be representable at all. No breaking changes to public APIs.
github-actions
Bot
added
sqllogictest
timsaucer
changed the title
50 tasks
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## branch-55 #24377 +/- ##
=============================================
- Coverage 81.14% 81.14% -0.01%
=============================================
Files 1110 1110
Lines 386184 386305 +121
Branches 386184 386305 +121
=============================================
+ Hits 313372 313469 +97
- Misses 54343 54361 +18
- Partials 18469 18475 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
alamb
approved these changes
Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a back port of #24365 into
branch-55