[AnuragRaut08]

Rationale for this change

MapArray validation uses MayHaveNulls() to check that the map child and keys contain no nulls. When an all-valid validity bitmap is present with an unknown null count, MayHaveNulls() can report that the array may contain nulls even though all values are valid.

During a cast of a MapArray, this can cause validation to return an invalid status which is passed to ARROW_CHECK_OK, resulting in a process abort instead of a recoverable error.

What changes are included in this PR?

• Use GetNullCount() instead of MayHaveNulls() when validating the MapArray child and keys.
• Add a regression test covering an all-valid validity bitmap with kUnknownNullCount.

Are these changes tested?

Yes, with existing and additional unit tests.

Are there any user-facing changes?

Only a bugfix.

This PR contains a "Critical Fix".

This fixes a bug that can cause a process crash when casting a valid MapArray whose keys carry an all-valid validity bitmap with an unknown null count. This is easily reproduced in Python:

import pyarrow as pa
m = pa.array([{"a": "1"}, {"b": "2"}], type=pa.map_(pa.string(), pa.string()))
m.filter(pa.array([False, True])).cast(pa.map_(pa.large_string(), pa.large_string()))

• GitHub Issue: [C++][Python] Casting a MapArray whose keys child carries an all-valid validity bitmap aborts the process (residual case of GH-38553) #51029

[github-actions]

⚠️ GitHub issue #51029 has been automatically assigned in GitHub to PR creator.

[pitrou]

Thanks for doing this @AnuragRaut08 ! See comments below.

Also, there are other uses of MayHaveNulls in array_nested.cc, shouldn't we fix them as well?

[Reranko05]

Also fix the lint by using:-
python -m pre_commit run clang-format --files <file_path>

[AnuragRaut08]

Done, ran clang-format on the affected C++ files with pre-commit.

[AnuragRaut08]

@pitrou I checked the other MayHaveNulls() uses in array_nested.cc. They are used for validating nullable list/map offsets when constructing arrays, rather than validating MapArray keys, so I left them unchanged.

[pitrou]

@pitrou I checked the other MayHaveNulls() uses in array_nested.cc. They are used for validating nullable list/map offsets when constructing arrays, rather than validating MapArray keys, so I left them unchanged.

But the same issue applies: MayHaveNulls does not tell whether there are really nulls. So we should strengthen those checks as well.

[AnuragRaut08]

Thanks, addressed. I have updated the remaining MayHaveNulls() checks in array_nested.cc to use GetNullCount() != 0 and added regression coverage for an all-valid offsets bitmap with kUnknownNullCount. The TestMapArray.* tests pass.

[pitrou]

Thank you, LGTM.

[pitrou]

Suggested change

build/

[pitrou]

@AlenkaF FYI