Document Union nullability contract#8713
Closed
connortsui20 wants to merge 1 commit into
Closed
Conversation
Merging this PR will not alter performance
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
d4ed595 to
728422f
Compare
This was referenced Jul 10, 2026
Member
Author
|
Superseded by #8714, which removes the stored Union nullability and derives it from the variants at runtime. |
connortsui20
added a commit
that referenced
this pull request
Jul 10, 2026
## Summary Tracking issue: #7882 A union has no independent parent validity. A row is null when its selected child is null, so the union's logical nullability is determined by its variants. Storing the same information on `DType::Union` created two sources of truth and allowed the outer value to disagree with the variants. This change makes inconsistent union DTypes impossible. The remaining question is how an operation such as `mask` should change a union's variants when it introduces nulls. That remains deferred and does not block the canonical sparse `UnionArray`. The Arrow implementation notes and deferred operation scope are in [this comment on #7882](#7882 (comment)). ## Changes - change `DType::Union` to store only `UnionVariants` - derive union nullability at runtime from the variant DTypes - name the non-zero-cost scan `derived_nullability` - leave `with_nullability` unchanged for unions instead of rewriting variant schemas - restrict union least-supertype coercion to identical variants for now - remove Union nullability from Serde, protobuf, and FlatBuffers Supersedes #8713. --------- Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
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.
Rationale
The current
developcontract is sufficient to start the canonical sparseUnionArray: Vortex's union-level nullability is a type-level summary derived from its variants, while a concrete row's validity comes from its selected child. There is no parent validity bitmap.What is not yet settled is how a generic operation should change a union's declared nullability—for example, whether
maskshould make an existing variant nullable, require aNullvariant, or take an explicit output type. That API question does not need to block the array layout.This PR therefore documents the existing contract and the boundary of the deferred work rather than changing behavior.
Arrow implementation survey
The implementations agree on the portable row-level semantic rule, but expose physical and logical nulls differently:
is_null/null_countreport false/zero, whilelogical_nulls()gathers validity from the selected child. ItsField::new_unionhelper makes the outer field non-nullable.IsNull(i)follows the selected child andComputeLogicalNullCount()counts selected-child nulls.The stable cross-implementation rule is therefore: the selected child determines whether a union row is logically null, and inactive sparse-child nulls are ignored. Outer Arrow
Field::nullableconventions should not define Vortex's contract.The investigation behind this boundary is in this comment on #7882.
Changes
DType::Unionnullability constraintDType::with_nullabilitycurrently changes only the outer summary and can create an inconsistent union DTypeDType::Union#7882 defines themInitial implementation boundary
The canonical sparse array can now proceed with predeclared, internally consistent union DTypes and DType-preserving operations such as scalar access, slice, filter, and take with non-nullable indices.
mask, nullable-index take, casts that introduce nulls, and null-appending builder behavior remain explicit follow-up work.Verification
cargo +nightly fmt --all --checkcargo test -p vortex-array --doc