diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc index 392fd9fbb70..18e446ce082 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc @@ -384,17 +384,58 @@ 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()); + 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) { + 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 51e6ca534cd..054e505583b 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_test.cc @@ -4145,6 +4145,128 @@ 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, 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"([ + {"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)}); + + 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)}); + 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 1e08e73668e..61a117734b9 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)