GH-49889: [C++][Compute] Handle logical nulls in validity kernels

[hadrian-reppas]

Rationale for this change

I was looking into #49889 and updated the handling of logical nulls in the is_valid, is_null, and true_unless_null kernels. It turns out there is some prior work here that I didn't see before I starting implementing my fix (#35058, #35036, #37642). For some reason, that work has stalled.

What changes are included in this PR?

This PR updates the IsValidExec, IsNullExec, and TrueUnlessNullExec kernels so that they call out to a new function SetLogicalNullBits. The SetLogicalNullBits function matches the logic in ArraySpan::ComputeLogicalNullCount(). The logic is similar enough that they may be some opportunities for code deduplication in dict_util.cc and ree_util.cc.

Are these changes tested?

Yes

Are there any user-facing changes?

No

• GitHub Issue: [C++][Compute] true_unless_null kernel output incorrect when run-end encoded array contains null #49889

[github-actions]

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

• Other pull requests
• Contribution Guidelines - Contributing Overview

[pitrou]

This is looking promising @hadrian-reppas !

[pitrou]

Using GetNullCount implies we're not looking up logical nulls in the dictionary values, only physical nulls, right?

This may be ok given that ArraySpan::ComputeLogicalNullCount does the same, but perhaps add a comment here?

[hadrian-reppas]

Yeah this feels kind of odd. I think we should probably be calling ComputeLogicalNullCount in both LogicalNullCount and SetLogicalNullBits.

[hadrian-reppas]

Actually, things become kind of messy if you look at logical nulls in the dictionary. Right now, it just looks at the dictionary validity bitmap. But if we wanted to properly handle logical nulls, we would need to deal with dictionaries that are dictionary, ree, and union types. So I'll keep this behavior for now and add a comment.

[pitrou]

I think you're right, also, dict_util::LogicalNullCount does not account for logical nulls in the dictionary either.

[hadrian-reppas]

I just addressed your comments. Let me know what you think, @pitrou.

[pitrou]

(thanks a lot for the update btw, apart from the comment above it LGTM)