Skip to content

Document Union nullability contract#8713

Closed
connortsui20 wants to merge 1 commit into
developfrom
codex/union-contract-correction
Closed

Document Union nullability contract#8713
connortsui20 wants to merge 1 commit into
developfrom
codex/union-contract-correction

Conversation

@connortsui20

@connortsui20 connortsui20 commented Jul 10, 2026

Copy link
Copy Markdown
Member

Rationale

The current develop contract is sufficient to start the canonical sparse UnionArray: 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 mask should make an existing variant nullable, require a Null variant, 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:

  • arrow-rs has no parent null buffer; physical is_null/null_count report false/zero, while logical_nulls() gathers validity from the selected child. Its Field::new_union helper makes the outer field non-nullable.
  • Arrow C++ also has no parent bitmap; IsNull(i) follows the selected child and ComputeLogicalNullCount() counts selected-child nulls.
  • Arrow Java reports no physical parent nulls, while value access follows the selected child. Its dense and sparse helpers are not consistent about default outer field nullability.

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::nullable conventions should not define Vortex's contract.

The investigation behind this boundary is in this comment on #7882.

Changes

  • document the existing variant-derived DType::Union nullability constraint
  • document selected-child logical validity and the lack of a parent bitmap
  • call out that DType::with_nullability currently changes only the outer summary and can create an inconsistent union DType
  • require initial union operations to reject nullability-changing cases until Tracking Issue: Add DType::Union #7882 defines them

Initial 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 --check
  • cargo test -p vortex-array --doc
  • pre-push Clippy checks

@connortsui20 connortsui20 added the changelog/fix A bug fix label Jul 10, 2026
@codspeed-hq

codspeed-hq Bot commented Jul 10, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

⚡ 3 improved benchmarks
❌ 1 regressed benchmark
✅ 1626 untouched benchmarks
⏩ 42 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation compact_sliced[(4096, 90)] 779.7 ns 867.2 ns -10.09%
Simulation bitwise_not_vortex_buffer_mut[128] 273.6 ns 244.4 ns +11.93%
Simulation compact[(4096, 90)] 867.2 ns 779.7 ns +11.22%
Simulation compare_int_constant 298.4 µs 268.5 µs +11.12%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing codex/union-contract-correction (728422f) with develop (42bea5d)

Open in CodSpeed

Footnotes

  1. 42 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20 connortsui20 force-pushed the codex/union-contract-correction branch from d4ed595 to 728422f Compare July 10, 2026 13:40
@connortsui20 connortsui20 changed the title Correct Union type invariants Document Union nullability contract Jul 10, 2026

Copy link
Copy Markdown
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant