Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 36 additions & 5 deletions cpp/src/arrow/compute/kernels/scalar_cast_nested.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand Down
118 changes: 118 additions & 0 deletions cpp/src/arrow/compute/kernels/scalar_cast_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<DataType> NestedNullabilityDestType() {
return struct_({field("inner", struct_({field("a", int32(), /*nullable=*/false)}))});
}

// Build struct<inner: struct<a: int32>> 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<std::shared_ptr<Array>> MakeNestedNullabilityArray(
const std::string& a_json, const std::vector<int>& inner_is_valid,
const std::vector<int>& outer_is_valid) {
std::shared_ptr<Buffer> 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<Array>& src,
const std::shared_ptr<DataType>& 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, "[]"));
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also check with a zero-length input? (not sure whether it should succeed or fail)


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<Buffer> outer_bitmap;
BitmapFromVector<int>({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<std::string> src_field_names = {"a", "b"};
std::shared_ptr<Array> a, b;
Expand Down
35 changes: 35 additions & 0 deletions cpp/src/arrow/util/bit_util_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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}) {
Expand Down
16 changes: 16 additions & 0 deletions cpp/src/arrow/util/bitmap_ops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down
14 changes: 14 additions & 0 deletions cpp/src/arrow/util/bitmap_ops.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
44 changes: 44 additions & 0 deletions python/pyarrow/tests/test_compute.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down