From 379ee41121420734fc3f7fbf20d8e2efb2888923 Mon Sep 17 00:00:00 2001 From: Aaron Sequeira Date: Sun, 19 Jul 2026 14:46:11 +0300 Subject: [PATCH 1/3] GH-50515: [C++][Compute] Respect parent validity in nested struct 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. --- .../compute/kernels/scalar_cast_nested.cc | 45 ++++++++- .../arrow/compute/kernels/scalar_cast_test.cc | 97 +++++++++++++++++++ python/pyarrow/tests/test_compute.py | 43 ++++++++ 3 files changed, 181 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc index 392fd9fbb705..47dd4578c2b1 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc @@ -29,6 +29,7 @@ #include "arrow/compute/kernels/common_internal.h" #include "arrow/compute/kernels/scalar_cast_internal.h" #include "arrow/util/bitmap_ops.h" +#include "arrow/util/bit_block_counter.h" #include "arrow/util/int_util.h" #include "arrow/util/logging_internal.h" @@ -384,12 +385,48 @@ struct CastStruct { const auto& in_field = in_type.field(in_field_index); const auto& in_values = (in_array.child_data[in_field_index].ToArrayData()->Slice( in_array.offset, in_array.length)); + if (in_field->nullable() && !out_field->nullable() && in_values->GetNullCount() > 0) { - return Status::Invalid( - "field '", in_field->name(), "' of type ", in_field->type()->ToString(), - " has nulls. Can't cast to non-nullable field '", out_field->name(), - "' of type ", out_field_type->ToString()); + bool has_nulls = false; + const uint8_t* parent_bitmap = in_array.buffers[0].data; + const uint8_t* child_bitmap = in_values->buffers.empty() ? nullptr : (in_values->buffers[0] ? in_values->buffers[0]->data() : nullptr); + + if (parent_bitmap == nullptr) { + // Parent has no nulls. Since child has nulls, they are unmasked. + has_nulls = true; + } else if (child_bitmap == nullptr) { + // 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. + for (int64_t i = 0; i < in_array.length; ++i) { + if (arrow::bit_util::GetBit(parent_bitmap, in_array.offset + i) && + in_values->IsNull(i)) { + has_nulls = true; + break; + } + } + } else { + // Both parent and child have bitmaps. Check if parent is valid AND child is null. + arrow::internal::BinaryBitBlockCounter bit_counter( + parent_bitmap, in_array.offset, + child_bitmap, in_values->offset, in_array.length); + int64_t position = 0; + while (position < in_array.length) { + arrow::internal::BitBlockCount block = bit_counter.NextAndNotWord(); + if (block.popcount > 0) { + has_nulls = true; + break; + } + position += block.length; + } + } + + if (has_nulls) { + return Status::Invalid( + "field '", in_field->name(), "' of type ", in_field->type()->ToString(), + " has nulls. Can't cast to non-nullable field '", out_field->name(), + "' of type ", out_field_type->ToString()); + } } ARROW_ASSIGN_OR_RAISE(Datum cast_values, Cast(in_values, out_field_type, options, ctx->exec_context())); diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_test.cc b/cpp/src/arrow/compute/kernels/scalar_cast_test.cc index 51e6ca534cd5..56a5b655e594 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_test.cc @@ -4145,6 +4145,103 @@ TEST(Cast, StructToStructSubsetWithNulls) { CheckStructToStructSubsetWithNulls(NumericTypes()); } +TEST(Cast, StructNestedNullabilityAbsentParent) { + auto inner_type_dest = struct_({field("a", int32(), /*nullable=*/false)}); + auto outer_type_dest = struct_({field("inner", inner_type_dest)}); + auto inner_type_src = struct_({field("a", int32())}); + auto outer_type_src = struct_({field("inner", inner_type_src)}); + + auto src = ArrayFromJSON(outer_type_src, R"([ + {"inner": {"a": 1}}, + {"inner": {"a": null}} + ])"); + EXPECT_RAISES_WITH_MESSAGE_THAT( + Invalid, ::testing::HasSubstr("has nulls. Can't cast to non-nullable field"), + Cast(src, CastOptions::Safe(outer_type_dest))); +} + +TEST(Cast, StructNestedNullabilityMasked) { + auto inner_type_dest = struct_({field("a", int32(), /*nullable=*/false)}); + auto outer_type_dest = struct_({field("inner", inner_type_dest)}); + auto inner_type_src = struct_({field("a", int32())}); + auto outer_type_src = struct_({field("inner", inner_type_src)}); + + auto src = ArrayFromJSON(outer_type_src, R"([ + {"inner": {"a": 1}}, + null + ])"); + auto expected = ArrayFromJSON(outer_type_dest, R"([ + {"inner": {"a": 1}}, + null + ])"); + CheckCast(src, expected); +} + +TEST(Cast, StructNestedNullabilitySliced) { + auto inner_type_dest = struct_({field("a", int32(), /*nullable=*/false)}); + auto outer_type_dest = struct_({field("inner", inner_type_dest)}); + auto inner_type_src = struct_({field("a", int32())}); + auto outer_type_src = struct_({field("inner", inner_type_src)}); + + auto src = ArrayFromJSON(outer_type_src, R"([ + {"inner": {"a": 1}}, + {"inner": {"a": 2}}, + {"inner": {"a": null}}, + null, + {"inner": {"a": 5}} + ])"); + auto expected = ArrayFromJSON(outer_type_dest, R"([ + {"inner": {"a": 1}}, + {"inner": {"a": 2}}, + {"inner": {"a": null}}, + null, + {"inner": {"a": 5}} + ])"); + + CheckCast(src->Slice(3, 2), expected->Slice(3, 2)); + + EXPECT_RAISES_WITH_MESSAGE_THAT( + Invalid, ::testing::HasSubstr("has nulls. Can't cast to non-nullable field"), + Cast(src->Slice(2, 2), CastOptions::Safe(outer_type_dest))); +} + +TEST(Cast, StructNestedNullabilityAbsentChild) { + auto inner_type_dest = struct_({field("a", int32(), /*nullable=*/false)}); + auto outer_type_dest = struct_({field("inner", inner_type_dest)}); + auto inner_type_src = struct_({field("a", int32())}); + auto outer_type_src = struct_({field("inner", inner_type_src)}); + + auto src = ArrayFromJSON(outer_type_src, R"([ + {"inner": {"a": 1}}, + {"inner": {"a": 2}} + ])"); + auto expected = ArrayFromJSON(outer_type_dest, R"([ + {"inner": {"a": 1}}, + {"inner": {"a": 2}} + ])"); + CheckCast(src, expected); +} + +TEST(Cast, StructNestedNullabilityDeep) { + auto deep_inner_dest = struct_({field("a", int32(), /*nullable=*/false)}); + auto deep_mid_dest = struct_({field("mid", deep_inner_dest)}); + auto deep_outer_dest = struct_({field("outer", deep_mid_dest)}); + + auto deep_inner_src = struct_({field("a", int32())}); + auto deep_mid_src = struct_({field("mid", deep_inner_src)}); + auto deep_outer_src = struct_({field("outer", deep_mid_src)}); + + auto src = ArrayFromJSON(deep_outer_src, R"([ + {"outer": {"mid": {"a": 1}}}, + null + ])"); + auto expected = ArrayFromJSON(deep_outer_dest, R"([ + {"outer": {"mid": {"a": 1}}}, + null + ])"); + CheckCast(src, expected); +} + TEST(Cast, StructToSameSizedButDifferentNamedStruct) { std::vector src_field_names = {"a", "b"}; std::shared_ptr a, b; diff --git a/python/pyarrow/tests/test_compute.py b/python/pyarrow/tests/test_compute.py index 1e08e73668e5..9ba41bdeba56 100644 --- a/python/pyarrow/tests/test_compute.py +++ b/python/pyarrow/tests/test_compute.py @@ -2395,6 +2395,49 @@ def check_cast_float_to_decimal(float_ty, float_val, decimal_ty, decimal_ctx, f"diff_digits = {diff_digits!r}") +def test_cast_struct_nested_nullability(): + # Arrow #50515: casting nested structs with non-nullable inner fields + # when the parent struct is nullable and marked as null + + # Target schema: outer struct is nullable, inner struct has non-nullable field + target_type = pa.struct([ + pa.field('inner', pa.struct([ + pa.field('a', pa.int32(), nullable=False) + ]), nullable=True) + ]) + + # Source array: + # [0] inner: {a: 1} + # [1] null + arr = pa.array([{'inner': {'a': 1}}, None]) + + # Casting should succeed, propagating the null appropriately + result = pc.cast(arr, target_type) + assert result.to_pylist() == [{'inner': {'a': 1}}, None] + + # True violation: outer struct is valid, but inner struct has null field 'a' + arr_fail = pa.array([{'inner': {'a': 1}}, {'inner': {'a': None}}]) + with pytest.raises(pa.ArrowInvalid, match="has nulls"): + pc.cast(arr_fail, target_type) + + # Slice test: a larger array sliced to only include masked nulls + arr_large = pa.array([ + {'inner': {'a': 1}}, + {'inner': {'a': None}}, + None, + {'inner': {'a': 5}} + ]) + + # Slice [2:4] includes `None, {'inner': {'a': 5}}` + # The true violation at index 1 is excluded. MUST succeed. + result_sliced = pc.cast(arr_large.slice(2, 2), target_type) + assert result_sliced.to_pylist() == [None, {'inner': {'a': 5}}] + + # Slice [1:3] includes `{'inner': {'a': None}}, None` + # The true violation at index 1 is included. MUST fail. + with pytest.raises(pa.ArrowInvalid, match="has nulls"): + pc.cast(arr_large.slice(1, 2), target_type) + # Cannot test float32 as case generators above assume float64 @pytest.mark.numpy @pytest.mark.parametrize('float_ty', [pa.float64()], ids=str) From 5fe5a33b2a6db407321e3cb4a32b7bc33e276576 Mon Sep 17 00:00:00 2001 From: Aaron Sequeira Date: Thu, 23 Jul 2026 17:25:40 +0000 Subject: [PATCH 2/3] GH-50515: Address review feedback on nested struct nullability cast - Replace the manual BinaryBitBlockCounter loop with CountSetBits/ CountAndSetBits for the case where both parent and child have validity bitmaps. - Conservatively reject (rather than doing a per-element logical-null check) when the child has no validity buffer of its own, e.g. NullType, RunEndEncoded, or Union, since a bitmap-only check can't distinguish masked from unmasked nulls there anyway. - Strip the validity buffer from a non-nullable field's cast output so masked-but-physically-null child data doesn't leak through if that field is later extracted on its own. - Add a regression test covering the no-child-bitmap path, rename the test that didn't actually exercise it, and fix a couple of lint nits (an overlong line and a missing blank line before the next test). --- .../compute/kernels/scalar_cast_nested.cc | 62 ++++++++++--------- .../arrow/compute/kernels/scalar_cast_test.cc | 29 ++++++++- python/pyarrow/tests/test_compute.py | 1 + 3 files changed, 61 insertions(+), 31 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc index 47dd4578c2b1..18e446ce0820 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc @@ -29,7 +29,6 @@ #include "arrow/compute/kernels/common_internal.h" #include "arrow/compute/kernels/scalar_cast_internal.h" #include "arrow/util/bitmap_ops.h" -#include "arrow/util/bit_block_counter.h" #include "arrow/util/int_util.h" #include "arrow/util/logging_internal.h" @@ -388,40 +387,35 @@ struct CastStruct { if (in_field->nullable() && !out_field->nullable() && in_values->GetNullCount() > 0) { - bool has_nulls = false; const uint8_t* parent_bitmap = in_array.buffers[0].data; - const uint8_t* child_bitmap = in_values->buffers.empty() ? nullptr : (in_values->buffers[0] ? in_values->buffers[0]->data() : nullptr); + const uint8_t* child_bitmap = + in_values->buffers.empty() || !in_values->buffers[0] + ? nullptr + : in_values->buffers[0]->data(); + int64_t unmasked_null_count; if (parent_bitmap == nullptr) { - // Parent has no nulls. Since child has nulls, they are unmasked. - has_nulls = true; + // Parent has no nulls, so any child null is unmasked. + unmasked_null_count = in_values->GetNullCount(); } else if (child_bitmap == nullptr) { - // 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. - for (int64_t i = 0; i < in_array.length; ++i) { - if (arrow::bit_util::GetBit(parent_bitmap, in_array.offset + i) && - in_values->IsNull(i)) { - has_nulls = true; - break; - } - } + // Child reports nulls but has no validity buffer of its own (e.g. + // NullArray, RunEndEncoded, Union). There is no cheap, bitmap-only way + // to tell which of those nulls are masked by the parent, so + // conservatively reject rather than reason about logical nulls (which + // would be inconsistent with the bitmap-only check below). + unmasked_null_count = in_values->GetNullCount(); } else { - // Both parent and child have bitmaps. Check if parent is valid AND child is null. - arrow::internal::BinaryBitBlockCounter bit_counter( - parent_bitmap, in_array.offset, - child_bitmap, in_values->offset, in_array.length); - int64_t position = 0; - while (position < in_array.length) { - arrow::internal::BitBlockCount block = bit_counter.NextAndNotWord(); - if (block.popcount > 0) { - has_nulls = true; - break; - } - position += block.length; - } + // Both parent and child have bitmaps: an unmasked null is a position + // where the parent is valid but the child is null, i.e. parent bits + // set that aren't also set in the child. + unmasked_null_count = arrow::internal::CountSetBits( + parent_bitmap, in_array.offset, in_array.length) - + arrow::internal::CountAndSetBits( + parent_bitmap, in_array.offset, child_bitmap, + in_values->offset, in_array.length); } - if (has_nulls) { + if (unmasked_null_count > 0) { return Status::Invalid( "field '", in_field->name(), "' of type ", in_field->type()->ToString(), " has nulls. Can't cast to non-nullable field '", out_field->name(), @@ -431,7 +425,17 @@ struct CastStruct { ARROW_ASSIGN_OR_RAISE(Datum cast_values, Cast(in_values, out_field_type, options, ctx->exec_context())); DCHECK(cast_values.is_array()); - out_array->child_data.push_back(cast_values.array()); + auto cast_array_data = cast_values.array(); + if (!out_field->nullable() && cast_array_data->buffers[0] != nullptr) { + // The child may still carry nulls that were only masked by the parent's + // validity bitmap (see the check above). Strip them so a non-nullable + // field never exposes a validity buffer -- otherwise extracting this + // field on its own (e.g. via struct_field/flatten) would surface nulls + // in a field whose type claims none are possible. + cast_array_data->buffers[0] = nullptr; + cast_array_data->null_count = 0; + } + out_array->child_data.push_back(std::move(cast_array_data)); } } diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_test.cc b/cpp/src/arrow/compute/kernels/scalar_cast_test.cc index 56a5b655e594..054e505583be 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_test.cc @@ -4197,7 +4197,7 @@ TEST(Cast, StructNestedNullabilitySliced) { null, {"inner": {"a": 5}} ])"); - + CheckCast(src->Slice(3, 2), expected->Slice(3, 2)); EXPECT_RAISES_WITH_MESSAGE_THAT( @@ -4205,7 +4205,7 @@ TEST(Cast, StructNestedNullabilitySliced) { Cast(src->Slice(2, 2), CastOptions::Safe(outer_type_dest))); } -TEST(Cast, StructNestedNullabilityAbsentChild) { +TEST(Cast, StructNestedNullabilityNoChildNulls) { auto inner_type_dest = struct_({field("a", int32(), /*nullable=*/false)}); auto outer_type_dest = struct_({field("inner", inner_type_dest)}); auto inner_type_src = struct_({field("a", int32())}); @@ -4222,6 +4222,31 @@ TEST(Cast, StructNestedNullabilityAbsentChild) { CheckCast(src, expected); } +TEST(Cast, StructNestedNullabilityNoChildBitmapConservativelyRejected) { + // NullType (and other child types without a validity buffer of their own, + // e.g. RunEndEncoded/Union) have no bitmap to distinguish masked from + // unmasked nulls, so any reported null is conservatively rejected -- even + // when the parent row is itself null and the value would otherwise be + // masked. + auto inner_type_dest = struct_({field("a", null(), /*nullable=*/false)}); + auto outer_type_dest = struct_({field("inner", inner_type_dest)}); + auto inner_type_src = struct_({field("a", null())}); + auto outer_type_src = struct_({field("inner", inner_type_src)}); + + auto src = ArrayFromJSON(outer_type_src, R"([ + {"inner": {"a": null}}, + null + ])"); + EXPECT_RAISES_WITH_MESSAGE_THAT( + Invalid, ::testing::HasSubstr("has nulls. Can't cast to non-nullable field"), + Cast(src, CastOptions::Safe(outer_type_dest))); + + // Slicing to only the masked (fully-null outer) row is still rejected. + EXPECT_RAISES_WITH_MESSAGE_THAT( + Invalid, ::testing::HasSubstr("has nulls. Can't cast to non-nullable field"), + Cast(src->Slice(1, 1), CastOptions::Safe(outer_type_dest))); +} + TEST(Cast, StructNestedNullabilityDeep) { auto deep_inner_dest = struct_({field("a", int32(), /*nullable=*/false)}); auto deep_mid_dest = struct_({field("mid", deep_inner_dest)}); diff --git a/python/pyarrow/tests/test_compute.py b/python/pyarrow/tests/test_compute.py index 9ba41bdeba56..61a117734b9a 100644 --- a/python/pyarrow/tests/test_compute.py +++ b/python/pyarrow/tests/test_compute.py @@ -2438,6 +2438,7 @@ def test_cast_struct_nested_nullability(): with pytest.raises(pa.ArrowInvalid, match="has nulls"): pc.cast(arr_large.slice(1, 2), target_type) + # Cannot test float32 as case generators above assume float64 @pytest.mark.numpy @pytest.mark.parametrize('float_ty', [pa.float64()], ids=str) From da10b42b4df90b64aa302c49563cfad28ff648b5 Mon Sep 17 00:00:00 2001 From: Aaron Sequeira Date: Tue, 28 Jul 2026 10:06:04 +0300 Subject: [PATCH 3/3] GH-50515: Add CountAndNotSetBits and address review feedback - Add arrow::internal::CountAndNotSetBits to bitmap_ops, replacing the CountSetBits/CountAndSetBits subtraction in the struct cast kernel with a single "parent valid AND child null" count, plus a direct unit test. - Drop the "no cheap way to tell" reasoning for bitmap-less children: GetNullCount() only counts physical nulls, so union and run-end encoded children report 0 and never reach the check. NullType is the only type that does, and it is always rejected. Comments updated accordingly. - Build the nested-nullability test inputs with StructArray::Make and explicit validity bitmaps so where "a" physically holds nulls is pinned down by the test instead of ArrayFromJSON's layout choices. - Cover zero-length input on both the int32 and NullType paths: an empty slice reports no nulls, so it succeeds even at the offset of a null that the full array is rejected for. - Strip trailing whitespace in test_compute.py (autopep8 lint failure). --- .../compute/kernels/scalar_cast_nested.cc | 44 ++--- .../arrow/compute/kernels/scalar_cast_test.cc | 174 +++++++++--------- cpp/src/arrow/util/bit_util_test.cc | 35 ++++ cpp/src/arrow/util/bitmap_ops.cc | 16 ++ cpp/src/arrow/util/bitmap_ops.h | 14 ++ python/pyarrow/tests/test_compute.py | 12 +- 6 files changed, 173 insertions(+), 122 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc index 18e446ce0820..ad9539ebac9f 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc @@ -387,35 +387,25 @@ struct CastStruct { if (in_field->nullable() && !out_field->nullable() && in_values->GetNullCount() > 0) { + // A child null is only observable if the parent element is valid: nulls + // sitting under a null parent are masked and don't violate the + // non-nullable output field. + // + // GetNullCount() only counts physical nulls, so a child without a + // validity bitmap reports 0 here (unions and run-end encoded arrays + // included) and never reaches this branch -- except NullType, which is + // all-null by construction and has no bitmap to intersect against the + // parent's, so it is always rejected. const uint8_t* parent_bitmap = in_array.buffers[0].data; const uint8_t* child_bitmap = - in_values->buffers.empty() || !in_values->buffers[0] - ? nullptr - : in_values->buffers[0]->data(); - - int64_t unmasked_null_count; - if (parent_bitmap == nullptr) { - // Parent has no nulls, so any child null is unmasked. - unmasked_null_count = in_values->GetNullCount(); - } else if (child_bitmap == nullptr) { - // Child reports nulls but has no validity buffer of its own (e.g. - // NullArray, RunEndEncoded, Union). There is no cheap, bitmap-only way - // to tell which of those nulls are masked by the parent, so - // conservatively reject rather than reason about logical nulls (which - // would be inconsistent with the bitmap-only check below). - unmasked_null_count = in_values->GetNullCount(); - } else { - // Both parent and child have bitmaps: an unmasked null is a position - // where the parent is valid but the child is null, i.e. parent bits - // set that aren't also set in the child. - unmasked_null_count = arrow::internal::CountSetBits( - parent_bitmap, in_array.offset, in_array.length) - - arrow::internal::CountAndSetBits( - parent_bitmap, in_array.offset, child_bitmap, - in_values->offset, in_array.length); - } - - if (unmasked_null_count > 0) { + in_values->buffers[0] ? in_values->buffers[0]->data() : nullptr; + + const bool has_unmasked_nulls = + parent_bitmap == nullptr || child_bitmap == nullptr || + arrow::internal::CountAndNotSetBits(parent_bitmap, in_array.offset, + child_bitmap, in_values->offset, + in_array.length) > 0; + if (has_unmasked_nulls) { return Status::Invalid( "field '", in_field->name(), "' of type ", in_field->type()->ToString(), " has nulls. Can't cast to non-nullable field '", out_field->name(), diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_test.cc b/cpp/src/arrow/compute/kernels/scalar_cast_test.cc index 054e505583be..c3ab5114b467 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_test.cc @@ -4145,126 +4145,122 @@ TEST(Cast, StructToStructSubsetWithNulls) { CheckStructToStructSubsetWithNulls(NumericTypes()); } -TEST(Cast, StructNestedNullabilityAbsentParent) { - auto inner_type_dest = struct_({field("a", int32(), /*nullable=*/false)}); - auto outer_type_dest = struct_({field("inner", inner_type_dest)}); - auto inner_type_src = struct_({field("a", int32())}); - auto outer_type_src = struct_({field("inner", inner_type_src)}); - - auto src = ArrayFromJSON(outer_type_src, R"([ - {"inner": {"a": 1}}, - {"inner": {"a": null}} - ])"); +// GH-50515: a null in a nested child field is only a violation of an out field +// declared non-nullable if the enclosing struct element is itself valid. +namespace { + +std::shared_ptr NestedNullabilityDestType() { + return struct_({field("inner", struct_({field("a", int32(), /*nullable=*/false)}))}); +} + +// Build struct> from an explicit "a" child and explicit +// validity bitmaps, so that where "a" physically holds nulls is pinned down by +// the test rather than left to ArrayFromJSON's layout choices. +Result> MakeNestedNullabilityArray( + const std::string& a_json, const std::vector& inner_is_valid, + const std::vector& outer_is_valid) { + std::shared_ptr inner_bitmap, outer_bitmap; + RETURN_NOT_OK(GetBitmapFromVector(inner_is_valid, &inner_bitmap)); + RETURN_NOT_OK(GetBitmapFromVector(outer_is_valid, &outer_bitmap)); + ARROW_ASSIGN_OR_RAISE(auto inner, + StructArray::Make({ArrayFromJSON(int32(), a_json)}, + {field("a", int32())}, inner_bitmap)); + return StructArray::Make({inner}, {field("inner", inner->type())}, outer_bitmap); +} + +void AssertNestedNullabilityRejected(const std::shared_ptr& src, + const std::shared_ptr& dest_type) { EXPECT_RAISES_WITH_MESSAGE_THAT( Invalid, ::testing::HasSubstr("has nulls. Can't cast to non-nullable field"), - Cast(src, CastOptions::Safe(outer_type_dest))); + Cast(src, CastOptions::Safe(dest_type))); } -TEST(Cast, StructNestedNullabilityMasked) { - auto inner_type_dest = struct_({field("a", int32(), /*nullable=*/false)}); - auto outer_type_dest = struct_({field("inner", inner_type_dest)}); - auto inner_type_src = struct_({field("a", int32())}); - auto outer_type_src = struct_({field("inner", inner_type_src)}); +} // namespace - auto src = ArrayFromJSON(outer_type_src, R"([ - {"inner": {"a": 1}}, - null - ])"); - auto expected = ArrayFromJSON(outer_type_dest, R"([ +TEST(Cast, StructNestedNullabilityUnmasked) { + // "a" is null at index 1 while both enclosing structs are valid there, so the + // null is observable and the cast must be rejected. + ASSERT_OK_AND_ASSIGN(auto src, MakeNestedNullabilityArray("[1, null]", {1, 1}, {1, 1})); + AssertNestedNullabilityRejected(src, NestedNullabilityDestType()); +} + +TEST(Cast, StructNestedNullabilityMasked) { + // "a" is null at index 1, but the enclosing structs are null there too, so the + // null is unobservable and the cast must succeed. + ASSERT_OK_AND_ASSIGN(auto src, MakeNestedNullabilityArray("[1, null]", {1, 0}, {1, 0})); + CheckCast(src, ArrayFromJSON(NestedNullabilityDestType(), R"([ {"inner": {"a": 1}}, null - ])"); - CheckCast(src, expected); + ])")); } TEST(Cast, StructNestedNullabilitySliced) { - auto inner_type_dest = struct_({field("a", int32(), /*nullable=*/false)}); - auto outer_type_dest = struct_({field("inner", inner_type_dest)}); - auto inner_type_src = struct_({field("a", int32())}); - auto outer_type_src = struct_({field("inner", inner_type_src)}); + ASSERT_OK_AND_ASSIGN( + auto src, MakeNestedNullabilityArray("[1, 2, null, null, 5]", {1, 1, 1, 0, 1}, + {1, 1, 1, 0, 1})); + auto dest_type = NestedNullabilityDestType(); - auto src = ArrayFromJSON(outer_type_src, R"([ - {"inner": {"a": 1}}, - {"inner": {"a": 2}}, - {"inner": {"a": null}}, - null, - {"inner": {"a": 5}} - ])"); - auto expected = ArrayFromJSON(outer_type_dest, R"([ - {"inner": {"a": 1}}, - {"inner": {"a": 2}}, - {"inner": {"a": null}}, + // [3, 5) holds only the null at index 3, which is masked, so the cast succeeds. + CheckCast(src->Slice(3, 2), ArrayFromJSON(dest_type, R"([ null, {"inner": {"a": 5}} - ])"); + ])")); - CheckCast(src->Slice(3, 2), expected->Slice(3, 2)); + // [2, 4) still holds the unmasked null at index 2. + AssertNestedNullabilityRejected(src->Slice(2, 2), dest_type); - EXPECT_RAISES_WITH_MESSAGE_THAT( - Invalid, ::testing::HasSubstr("has nulls. Can't cast to non-nullable field"), - Cast(src->Slice(2, 2), CastOptions::Safe(outer_type_dest))); + // A zero-length slice has nothing to observe, even one taken at the offset of + // the unmasked null. + CheckCast(src->Slice(2, 0), ArrayFromJSON(dest_type, "[]")); } TEST(Cast, StructNestedNullabilityNoChildNulls) { - auto inner_type_dest = struct_({field("a", int32(), /*nullable=*/false)}); - auto outer_type_dest = struct_({field("inner", inner_type_dest)}); - auto inner_type_src = struct_({field("a", int32())}); - auto outer_type_src = struct_({field("inner", inner_type_src)}); - - auto src = ArrayFromJSON(outer_type_src, R"([ + ASSERT_OK_AND_ASSIGN(auto src, MakeNestedNullabilityArray("[1, 2]", {1, 1}, {1, 1})); + CheckCast(src, ArrayFromJSON(NestedNullabilityDestType(), R"([ {"inner": {"a": 1}}, {"inner": {"a": 2}} - ])"); - auto expected = ArrayFromJSON(outer_type_dest, R"([ - {"inner": {"a": 1}}, - {"inner": {"a": 2}} - ])"); - CheckCast(src, expected); + ])")); } -TEST(Cast, StructNestedNullabilityNoChildBitmapConservativelyRejected) { - // NullType (and other child types without a validity buffer of their own, - // e.g. RunEndEncoded/Union) have no bitmap to distinguish masked from - // unmasked nulls, so any reported null is conservatively rejected -- even - // when the parent row is itself null and the value would otherwise be - // masked. - auto inner_type_dest = struct_({field("a", null(), /*nullable=*/false)}); - auto outer_type_dest = struct_({field("inner", inner_type_dest)}); - auto inner_type_src = struct_({field("a", null())}); - auto outer_type_src = struct_({field("inner", inner_type_src)}); +TEST(Cast, StructNestedNullabilityNullTypeChild) { + // NullType is all-null by construction and has no validity bitmap to + // intersect against the parent's, so its nulls can never be shown to be + // masked and the cast is always rejected. (Other bitmap-less types such as + // union and run-end encoded report no physical nulls at all, so they are + // unaffected by this check.) + auto src_type = struct_({field("inner", struct_({field("a", null())}))}); + auto dest_type = + struct_({field("inner", struct_({field("a", null(), /*nullable=*/false)}))}); - auto src = ArrayFromJSON(outer_type_src, R"([ + auto src = ArrayFromJSON(src_type, R"([ {"inner": {"a": null}}, null ])"); - EXPECT_RAISES_WITH_MESSAGE_THAT( - Invalid, ::testing::HasSubstr("has nulls. Can't cast to non-nullable field"), - Cast(src, CastOptions::Safe(outer_type_dest))); + AssertNestedNullabilityRejected(src, dest_type); - // Slicing to only the masked (fully-null outer) row is still rejected. - EXPECT_RAISES_WITH_MESSAGE_THAT( - Invalid, ::testing::HasSubstr("has nulls. Can't cast to non-nullable field"), - Cast(src->Slice(1, 1), CastOptions::Safe(outer_type_dest))); + // Slicing to only the row where the outer element is null doesn't help. + AssertNestedNullabilityRejected(src->Slice(1, 1), dest_type); + + // A zero-length slice reports no nulls, so it succeeds. + CheckCast(src->Slice(1, 0), ArrayFromJSON(dest_type, "[]")); } TEST(Cast, StructNestedNullabilityDeep) { - auto deep_inner_dest = struct_({field("a", int32(), /*nullable=*/false)}); - auto deep_mid_dest = struct_({field("mid", deep_inner_dest)}); - auto deep_outer_dest = struct_({field("outer", deep_mid_dest)}); - - auto deep_inner_src = struct_({field("a", int32())}); - auto deep_mid_src = struct_({field("mid", deep_inner_src)}); - auto deep_outer_src = struct_({field("outer", deep_mid_src)}); - - auto src = ArrayFromJSON(deep_outer_src, R"([ - {"outer": {"mid": {"a": 1}}}, - null - ])"); - auto expected = ArrayFromJSON(deep_outer_dest, R"([ - {"outer": {"mid": {"a": 1}}}, + // Wrap the two-level array in one more struct level, so the masked null is + // only reached by recursing rather than at the outermost cast. + ASSERT_OK_AND_ASSIGN(auto inner, + MakeNestedNullabilityArray("[1, null]", {1, 0}, {1, 0})); + std::shared_ptr outer_bitmap; + BitmapFromVector({1, 0}, &outer_bitmap); + ASSERT_OK_AND_ASSIGN( + auto src, + StructArray::Make({inner}, {field("outer", inner->type())}, outer_bitmap)); + + auto dest_type = struct_({field("outer", NestedNullabilityDestType())}); + CheckCast(src, ArrayFromJSON(dest_type, R"([ + {"outer": {"inner": {"a": 1}}}, null - ])"); - CheckCast(src, expected); + ])")); } TEST(Cast, StructToSameSizedButDifferentNamedStruct) { diff --git a/cpp/src/arrow/util/bit_util_test.cc b/cpp/src/arrow/util/bit_util_test.cc index a6af004f7c5b..714ffa8ccea7 100644 --- a/cpp/src/arrow/util/bit_util_test.cc +++ b/cpp/src/arrow/util/bit_util_test.cc @@ -50,6 +50,8 @@ namespace arrow { using internal::BitsetStack; using internal::CopyBitmap; +using internal::CountAndNotSetBits; +using internal::CountAndSetBits; using internal::CountSetBits; using internal::InvertBitmap; using internal::ReverseBitmap; @@ -327,6 +329,39 @@ TEST(BitUtilTests, TestCountSetBits) { } } +TEST(BitUtilTests, TestCountAndNotSetBits) { + const int kBufferSize = 1000; + alignas(8) uint8_t left[kBufferSize] = {0}; + alignas(8) uint8_t right[kBufferSize] = {0}; + const int buffer_bits = kBufferSize * 8; + + random_bytes(kBufferSize, 0, left); + random_bytes(kBufferSize, 1, right); + + for (const int64_t left_offset : {0, 1, 7, 63, 64, 129}) { + for (const int64_t right_offset : {0, 3, 8, 64, 100}) { + const int64_t length = buffer_bits - std::max(left_offset, right_offset); + int64_t expected = 0; + for (int64_t i = 0; i < length; ++i) { + expected += bit_util::GetBit(left, left_offset + i) && + !bit_util::GetBit(right, right_offset + i); + } + ASSERT_EQ(expected, + CountAndNotSetBits(left, left_offset, right, right_offset, length)) + << "left_offset = " << left_offset << ", right_offset = " << right_offset; + } + } + + // Zero length is well-defined and touches no memory + ASSERT_EQ(0, CountAndNotSetBits(left, 0, right, 0, 0)); + + // "x & ~y" complements "x & y" over the bits set in x + const int64_t length = buffer_bits - 64; + ASSERT_EQ(CountSetBits(left, 64, length), + CountAndSetBits(left, 64, right, 3, length) + + CountAndNotSetBits(left, 64, right, 3, length)); +} + TEST(BitUtilTests, TestSetBitsTo) { using bit_util::SetBitsTo; for (const auto fill_byte_int : {0x00, 0xff}) { diff --git a/cpp/src/arrow/util/bitmap_ops.cc b/cpp/src/arrow/util/bitmap_ops.cc index 33a95150d53b..92ecbbce9d7e 100644 --- a/cpp/src/arrow/util/bitmap_ops.cc +++ b/cpp/src/arrow/util/bitmap_ops.cc @@ -104,6 +104,22 @@ int64_t CountAndSetBits(const uint8_t* left_bitmap, int64_t left_offset, return count; } +int64_t CountAndNotSetBits(const uint8_t* left_bitmap, int64_t left_offset, + const uint8_t* right_bitmap, int64_t right_offset, + int64_t length) { + BinaryBitBlockCounter bit_counter(left_bitmap, left_offset, right_bitmap, right_offset, + length); + int64_t count = 0; + while (true) { + BitBlockCount block = bit_counter.NextAndNotWord(); + if (block.length == 0) { + break; + } + count += block.popcount; + } + return count; +} + namespace { enum class TransferMode : bool { Copy, Invert }; diff --git a/cpp/src/arrow/util/bitmap_ops.h b/cpp/src/arrow/util/bitmap_ops.h index a503b9ec03ec..d150cd331be7 100644 --- a/cpp/src/arrow/util/bitmap_ops.h +++ b/cpp/src/arrow/util/bitmap_ops.h @@ -132,6 +132,20 @@ int64_t CountAndSetBits(const uint8_t* left_bitmap, int64_t left_offset, const uint8_t* right_bitmap, int64_t right_offset, int64_t length); +/// Compute the number of 1's in the result of an "and not" (& ~) of two bitmaps +/// +/// \param[in] left_bitmap a packed LSB-ordered bitmap as a byte array +/// \param[in] left_offset a bitwise offset into the left bitmap +/// \param[in] right_bitmap a packed LSB-ordered bitmap as a byte array +/// \param[in] right_offset a bitwise offset into the right bitmap +/// \param[in] length the length of the bitmaps (must be the same) +/// +/// \return The number of bits set in the left bitmap but not in the right one +ARROW_EXPORT +int64_t CountAndNotSetBits(const uint8_t* left_bitmap, int64_t left_offset, + const uint8_t* right_bitmap, int64_t right_offset, + int64_t length); + ARROW_EXPORT bool BitmapEquals(const uint8_t* left, int64_t left_offset, const uint8_t* right, int64_t right_offset, int64_t length); diff --git a/python/pyarrow/tests/test_compute.py b/python/pyarrow/tests/test_compute.py index 61a117734b9a..4557eadfd260 100644 --- a/python/pyarrow/tests/test_compute.py +++ b/python/pyarrow/tests/test_compute.py @@ -2398,23 +2398,23 @@ def check_cast_float_to_decimal(float_ty, float_val, decimal_ty, decimal_ctx, def test_cast_struct_nested_nullability(): # Arrow #50515: casting nested structs with non-nullable inner fields # when the parent struct is nullable and marked as null - + # Target schema: outer struct is nullable, inner struct has non-nullable field target_type = pa.struct([ pa.field('inner', pa.struct([ pa.field('a', pa.int32(), nullable=False) ]), nullable=True) ]) - - # Source array: + + # Source array: # [0] inner: {a: 1} # [1] null arr = pa.array([{'inner': {'a': 1}}, None]) - + # Casting should succeed, propagating the null appropriately result = pc.cast(arr, target_type) assert result.to_pylist() == [{'inner': {'a': 1}}, None] - + # True violation: outer struct is valid, but inner struct has null field 'a' arr_fail = pa.array([{'inner': {'a': 1}}, {'inner': {'a': None}}]) with pytest.raises(pa.ArrowInvalid, match="has nulls"): @@ -2427,7 +2427,7 @@ def test_cast_struct_nested_nullability(): None, {'inner': {'a': 5}} ]) - + # Slice [2:4] includes `None, {'inner': {'a': 5}}` # The true violation at index 1 is excluded. MUST succeed. result_sliced = pc.cast(arr_large.slice(2, 2), target_type)