diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc index 392fd9fbb705..ad9539ebac9f 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc @@ -384,17 +384,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()); + // 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[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(), + "' of type ", out_field_type->ToString()); + } } 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 51e6ca534cd5..c3ab5114b467 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_test.cc @@ -4145,6 +4145,124 @@ TEST(Cast, StructToStructSubsetWithNulls) { CheckStructToStructSubsetWithNulls(NumericTypes()); } +// 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(dest_type))); +} + +} // namespace + +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 + ])")); +} + +TEST(Cast, StructNestedNullabilitySliced) { + 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(); + + // [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}} + ])")); + + // [2, 4) still holds the unmasked null at index 2. + AssertNestedNullabilityRejected(src->Slice(2, 2), dest_type); + + // 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) { + ASSERT_OK_AND_ASSIGN(auto src, MakeNestedNullabilityArray("[1, 2]", {1, 1}, {1, 1})); + CheckCast(src, ArrayFromJSON(NestedNullabilityDestType(), R"([ + {"inner": {"a": 1}}, + {"inner": {"a": 2}} + ])")); +} + +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(src_type, R"([ + {"inner": {"a": null}}, + null + ])"); + AssertNestedNullabilityRejected(src, dest_type); + + // 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) { + // 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 + ])")); +} + TEST(Cast, StructToSameSizedButDifferentNamedStruct) { std::vector src_field_names = {"a", "b"}; std::shared_ptr a, b; 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 1e08e73668e5..4557eadfd260 100644 --- a/python/pyarrow/tests/test_compute.py +++ b/python/pyarrow/tests/test_compute.py @@ -2395,6 +2395,50 @@ 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)