GH-34315: [C++] Correct is_null kernel for Union and RunEndEncoded logical nulls #37642
Conversation
…ded logical nulls
Co-authored-by: Weston Pace <weston.pace@gmail.com>
Co-authored-by: Weston Pace <weston.pace@gmail.com>
github-actions
bot
added
Component: C++
Component: Python
awaiting review
| } | ||
| } | ||
|
|
||
| static void SetSparseUnionLogicalNullBits(const ArraySpan& span, uint8_t* out_bitmap, |
There was a problem hiding this comment.
Would it be worth reusing the choose kernel for this case?
| } | ||
|
|
||
| template <typename RunEndCType> | ||
| void SetREELogicalNullBits(const ArraySpan& span, uint8_t* out_bitmap, |
There was a problem hiding this comment.
Here it seems we could trivially create an ree($index_type, bool), then return that directly or decode it using existing utilities
| int64_t out_offset) { | ||
| const auto* dense_union_type = | ||
| ::arrow::internal::checked_cast<const DenseUnionType*>(span.type); | ||
| DCHECK_LE(span.child_data.size(), 128); |
There was a problem hiding this comment.
Someday if we have more dense union utilities the same trick as for sparse unions could be used here
github-actions
bot
added
awaiting changes
github-actions
bot
added
the
Status: stale-warning
|
Thank you for your contribution. Unfortunately, this |
github-actions
bot
removed
the
Status: stale-warning
4 participants
Rationale for this change
Currently the is_null kernel always returns all-False for union and run-end-encoded types, since those don't have a top-level validity buffer. Update the kernel to take the logical nulls into account.
Are these changes tested?
Yes, both in Python and C++
Are there any user-facing changes?
Yes, this changes (corrects) the behaviour of the is_null kernel.
Closes #35036
pa.compute.is_null()returns incorrect answer for dense union arrays and segfaults for dense union scalars #34315