GH-50515: [C++][Compute] Respect parent validity bitmap when casting nested structs with non-nullable fields#50546
Conversation
…ct cast When casting a struct with a nullable->non-nullable field change, CastStruct::Exec checks GetNullCount() on the child array without considering the parent struct's validity bitmap. This rejects casts where the child's physical nulls are fully masked by parent-level nulls. Intersect the parent and child validity bitmaps before deciding whether unmasked nulls exist, using BinaryBitBlockCounter for the common case and explicit fallbacks for absent bitmaps.
|
|
You can use |
| } | ||
| position += block.length; | ||
| } | ||
| } |
There was a problem hiding this comment.
If we get here, I think we should make sure that the final casted child does not have a null bitmap, otherwise it might violate expectations for a nullable field (which are not well specified, unfortunately).
| // Child has nulls but no bitmap (e.g. NullArray, RunEndEncoded, Union). | ||
| // We must semantically check if any valid parent element corresponds to a null child. |
There was a problem hiding this comment.
The BinaryBitBlockCounter path below does not account for logical nulls, so I don't think this one should, either.
We can just hardcode an error for NullArray.
|
@aaron-seq @davlee1972 thanks for the issue and PR. This is a bit tricky as we have never formalized what the nullable flag means for non-trivial types, so I've started a discussion on the dev ML to make sure we don't go into the wrong direction here: |
Rationale for this change
When casting a nested struct whose inner field changes from nullable to
non-nullable,
CastStruct::Execcurrently callsin_values->GetNullCount() > 0without considering the parent struct'svalidity bitmap. This causes a spurious
ArrowInvaliderror when thechild's physical nulls are fully masked by null parent entries.
Reported in #50515.
What changes are included in this PR?
In
scalar_cast_nested.cc, replace the unconditional rejection with athree-way check:
BinaryBitBlockCounter::NextAndNotWord()to detect parent-valid-AND-child-null positions without heap allocation.
Are these changes tested?
Five new C++ test cases in
scalar_cast_test.cc:StructNestedNullabilityAbsentParent— true violation, must rejectStructNestedNullabilityMasked— masked null, must succeedStructNestedNullabilitySliced— offset correctness for both outcomesStructNestedNullabilityAbsentChild— no nulls, must succeedStructNestedNullabilityDeep— 3-level nesting with masked nullOne new Python test in
test_compute.py:test_cast_struct_nested_nullability— end-to-end PyArrow validationcovering success, rejection, and sliced array cases
Are there any user-facing changes?
Struct casts that were previously rejected with
ArrowInvalid: field '...' has nullswill now succeed when the nullsare masked by parent-level nulls. This is a correctness fix, not a
behavior change — the previous behavior was incorrect per the Arrow
columnar format specification.