shoemoney commented on PR #51000: URL: https://github.com/apache/arrow/pull/51000#issuecomment-5731101782
Switched to your approach in (3). The NaN loop and its per-index-type dispatch are gone, replaced by `is_null` on the dictionary values, `take` through the indices, and an OR into the output bitmap. The output already holds the inverted index validity bitmap at that point, so the take result's values bitmap goes in as is, no masking needed. `take` runs with bounds checking, so an out-of-range index now errors instead of reading past the dictionary. That also answers (2): a null dictionary value behind a valid index is reported as null now. New test `IsNullDictionaryNullValues` fails at index 1 on the previous commit and passes on this one. I added `IsNullDictionaryNanIsNullUnsignedIndices` too, since the explicit index-type dispatch that used to be covered is gone. The path still only runs for `nan_is_null=true` with a floating-point value type, so `is_null` with default options is unchanged for every dictionary type. Tell me if you would rather null dictionary values be reported for all dictionary types, that is a one-line widening of the condition. Rebased onto main, commit 1d2b266. Local run of `arrow-compute-scalar-utility-test`: 99 tests from 41 suites passed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
