Skip to content

Commit

Permalink
apacheGH-38553 : [C++] Replace null_count with MayHaveNulls in ListAr…
Browse files Browse the repository at this point in the history
…rayFromArray and MapArray (apache#41957)

### Rationale for this change

Offsets could have `null_count() == -1` (`kUnknownNullCount`) meaning that offsets might contain nulls that are not accounted for which can produce failures (apache#38553) when working with `ListArray` or `MapArray`. `null_count()` should be replaced with `MayHaveNulls()`.

### What changes are included in this PR?

`null_count` is replaced with `MayHaveNulls` in `ListArrayFromArray`,  `MapArray::FromArraysInternal` and `MapArray::ValidateChildData`. Some tests had to be updated.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* GitHub Issue: apache#38553

Authored-by: AlenkaF <[email protected]>
Signed-off-by: Felipe Oliveira Carvalho <[email protected]>
  • Loading branch information
AlenkaF authored and vibhatha committed Jun 5, 2024
1 parent 9d66dc2 commit e66b04f
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 6 deletions.
2 changes: 1 addition & 1 deletion cpp/src/arrow/array/array_list_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1383,7 +1383,7 @@ TEST_F(TestMapArray, FromArrays) {

// Null bitmap and offset with offset
ASSERT_RAISES(NotImplemented,
MapArray::FromArrays(offsets3->Slice(2), keys, items, pool_,
MapArray::FromArrays(offsets1->Slice(2), keys, items, pool_,
offsets3->data()->buffers[0]));
}

Expand Down
8 changes: 4 additions & 4 deletions cpp/src/arrow/array/array_nested.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ Result<std::shared_ptr<typename TypeTraits<TYPE>::ArrayType>> ListArrayFromArray
return Status::TypeError("List offsets must be ", OffsetArrowType::type_name());
}

if (null_bitmap != nullptr && offsets.null_count() > 0) {
if (null_bitmap != nullptr && offsets.data()->MayHaveNulls()) {
return Status::Invalid(
"Ambiguous to specify both validity map and offsets with nulls");
}
Expand Down Expand Up @@ -827,7 +827,7 @@ Result<std::shared_ptr<Array>> MapArray::FromArraysInternal(
return Status::Invalid("Map key and item arrays must be equal length");
}

if (null_bitmap != nullptr && offsets->null_count() > 0) {
if (null_bitmap != nullptr && offsets->data()->MayHaveNulls()) {
return Status::Invalid(
"Ambiguous to specify both validity map and offsets with nulls");
}
Expand Down Expand Up @@ -893,13 +893,13 @@ Status MapArray::ValidateChildData(
if (pair_data->type->id() != Type::STRUCT) {
return Status::Invalid("Map array child array should have struct type");
}
if (pair_data->null_count != 0) {
if (pair_data->MayHaveNulls()) {
return Status::Invalid("Map array child array should have no nulls");
}
if (pair_data->child_data.size() != 2) {
return Status::Invalid("Map array child array should have two fields");
}
if (pair_data->child_data[0]->null_count != 0) {
if (pair_data->child_data[0]->MayHaveNulls()) {
return Status::Invalid("Map array keys array should have no nulls");
}
return Status::OK();
Expand Down
14 changes: 13 additions & 1 deletion python/pyarrow/tests/test_array.py
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,18 @@ def test_list_array_types_from_arrays_fail(list_array_type, list_type_factory):
arr_slice.offsets, arr_slice.values, mask=arr_slice.is_null())


def test_map_cast():
# GH-38553
t = pa.map_(pa.int64(), pa.int64())
arr = pa.array([{1: 2}], type=t)
result = arr.cast(pa.map_(pa.int32(), pa.int64()))

t_expected = pa.map_(pa.int32(), pa.int64())
expected = pa.array([{1: 2}], type=t_expected)

assert result.equals(expected)


def test_map_labelled():
# ARROW-13735
t = pa.map_(pa.field("name", "string", nullable=False), "int64")
Expand Down Expand Up @@ -1105,7 +1117,7 @@ def test_map_from_arrays():

# error if null bitmap passed to sliced offset
msg2 = 'Null bitmap with offsets slice not supported.'
offsets = pa.array(offsets, pa.int32())
offsets = pa.array([0, 2, 2, 6], pa.int32())
with pytest.raises(pa.ArrowNotImplementedError, match=msg2):
pa.MapArray.from_arrays(offsets.slice(2), keys, items, pa.map_(
keys.type,
Expand Down

0 comments on commit e66b04f

Please sign in to comment.