Repository navigation
fix(arrow-array): check logical nulls, not is_nullable, for a non-nullable list field - #11178
Merged
Jefffrey merged 1 commit intoSep 25, 2026
Conversation
…lable list field GenericListArray::try_new and GenericListViewArray::try_new rejected a non-nullable field whenever values.is_nullable() returned true, which DictionaryArray does conservatively for keys with an all-valid null buffer or values that hold an unreferenced null. Check logical nulls, as FixedSizeListArray::try_new already does. Closes apache#6538
Jefffrey
approved these changes
Sep 25, 2026
Contributor
|
thanks @amitvijapur |
haohuaijin
pushed a commit
to haohuaijin/arrow-rs
that referenced
this pull request
Sep 25, 2026
…lable list field (apache#11178) # Which issue does this PR close? Closes apache#6538. # Rationale for this change `GenericListArray::try_new` and `GenericListViewArray::try_new` reject a non-nullable `field` whenever `values.is_nullable()` is true. `Array::is_nullable` may return `true` conservatively, and `DictionaryArray::is_nullable` does so when the keys carry a null buffer, even an all-valid one, or when the values hold a null that no key references. Such a dictionary has zero logical nulls and is rejected anyway. The `UnionArray` case in the issue no longer reproduces: apache#6540 made `UnionArray::is_nullable` ask its children. The dictionary case still does. # What changes are included in this PR? Both constructors test `values.logical_null_count() != 0` instead, which is what `FixedSizeListArray::try_new` already checks through `logical_nulls()`. The doc bullets are updated to match. # Are these changes tested? A test in each file builds a list over an `Int8DictionaryArray` whose keys have an all-valid null buffer under a non-nullable field, which failed before the change, and then one with a null key, which still fails with `cannot contain nulls`. `cargo test -p arrow-array` passes. # Are there any user-facing changes? A list or list-view array over a dictionary with no logical nulls can now be built under a non-nullable field. Arrays that hold a logical null are still rejected as before.
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
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.
Which issue does this PR close?
Closes #6538.
Rationale for this change
GenericListArray::try_newandGenericListViewArray::try_newreject a non-nullablefieldwhenevervalues.is_nullable()is true.Array::is_nullablemay returntrueconservatively, andDictionaryArray::is_nullabledoes so when the keys carry a null buffer, even an all-valid one, or when the values hold a null that no key references. Such a dictionary has zero logical nulls and is rejected anyway.The
UnionArraycase in the issue no longer reproduces: #6540 madeUnionArray::is_nullableask its children. The dictionary case still does.What changes are included in this PR?
Both constructors test
values.logical_null_count() != 0instead, which is whatFixedSizeListArray::try_newalready checks throughlogical_nulls(). The doc bullets are updated to match.Are these changes tested?
A test in each file builds a list over an
Int8DictionaryArraywhose keys have an all-valid null buffer under a non-nullable field, which failed before the change, and then one with a null key, which still fails withcannot contain nulls.cargo test -p arrow-arraypasses.Are there any user-facing changes?
A list or list-view array over a dictionary with no logical nulls can now be built under a non-nullable field. Arrays that hold a logical null are still rejected as before.