`MaskedArray::try_new` rejects an empty child with an all-true validity bitmap

[connortsui20]

What happened?

Array::<Masked>::try_new returns an error for a zero-length child whose validity is an all-true bitmap. The same empty child constructs without an error when its validity is NonNullable or AllValid. The same bitmap shape also constructs without an error at length 3.

len=0  child NonNullable               => Ok
len=0  child AllValid                  => Ok
len=0  child Array(all-true bitmap)    => Err(MaskedArray children must not have nulls)

len=3  child NonNullable               => Ok
len=3  child AllValid                  => Ok
len=3  child Array(all-true bitmap)    => Ok

A zero-length child holds no elements, so it holds no nulls. All three rows at len=0 describe the same array. The constructor accepts two of them and rejects the third.

The result is that construction depends on the representation of the child validity, and not on whether the child holds a null. A caller that wraps a child from another part of the system cannot predict the outcome, because at length 0 the choice of representation carries no information.

The error message is also wrong for this input. The child has no nulls.

Steps to reproduce

fn bitmap(n: usize) -> Validity {
    Validity::Array(BoolArray::new(BitBuffer::new_set(n), Validity::NonNullable).into_array())
}

#[test]
fn empty_child_with_bitmap_validity() {
    let child = PrimitiveArray::new(Buffer::<i32>::empty(), bitmap(0)).into_array();

    // Fails. The same child with `Validity::NonNullable` or `Validity::AllValid` succeeds.
    Array::<Masked>::try_new(child, bitmap(0)).unwrap();
}

Observed:

Err(Other error: MaskedArray children must not have nulls
    at ./src/arrays/masked/array.rs:63)

Expected: the constructor returns Ok, as it does for the other two representations of the same empty child.

Additional context

The root cause is ArrayRef::all_valid, which returns false for an empty array with a validity bitmap. A fix there corrects this constructor and makes every other caller consistent at length 0.

Root cause

vortex-array/src/array/erased.rs:303 takes the answer from the Min statistic of the validity bitmap:

Validity::Array(a) => Ok(a.statistics().compute_min::<bool>(ctx).unwrap_or(false)),

For booleans, min == true holds only when every bit is set. This is the correct test on a non-empty bitmap. compute_min returns None for an empty array, because a minimum over zero elements does not exist. unwrap_or(false) then reads this absence as evidence of a null.

The NonNullable and AllValid arms return true without a statistic, which is why those two rows of the matrix succeed.

StatsSetRef::compute_as (vortex-array/src/stats/array.rs:224) maps two outcomes to None:

• The statistic does not exist. This is the empty-array case.
• The statistic failed to compute. compute_as logs the error and discards it.

A fix must separate these two conditions. Returning true for an empty array, before the statistic is read, corrects the reported case. The wider question is what an unavailable statistic means. For a caller that guards an unchecked read, false is the safe answer. For a caller that enforces an invariant, false produces this defect.

all_invalid has the mirrored shape, !compute_max::<bool>(ctx).unwrap_or(true), and also returns false for an empty array.

Other all_valid callers, checked

No other caller turns this into an observable failure today. This list records what was checked, so the analysis does not need to be repeated:

Caller	Effect at length 0	Reachable
Masked::try_new and Masked::build	hard error	yes, this issue
Patched::from_array_and_patches	would error	no, Patches::new rejects empty indices
bitpack_decompress patch assert	would panic	no, same reason
is_constant	returns false	no difference, every empty array returns false
sequence::compress	skips compression	harmless on zero rows
decimal compare	takes the slow path	harmless on zero rows
file_stats Stat::Max	skips one aggregated statistic	minor, on the stats table column

Array::<Masked>::slice(0..0) returns a primitive array rather than a Masked array, so slicing does not produce the rejected shape.

[robert3005]

Looks like the base case in all_valid/all_invalid is reversed?

[connortsui20]

Well I don't actually know if that base case is intended behavior or not. Technically both all_valid and all_invalid both have an argument for being true/false for an empty array (this is a degenerate case)