Copilot commented on code in PR #51000:
URL: https://github.com/apache/arrow/pull/51000#discussion_r4101274323


##########
cpp/src/arrow/compute/kernels/scalar_validity.cc:
##########
@@ -80,10 +83,43 @@ static void SetNanBits(const ArraySpan& arr, uint8_t* 
out_bitmap, int64_t out_of
   }
 }
 
+// Maps `is_null` over dictionary values and then through checked indices, so
+// both NaN and null dictionary entries are reported, whatever the index type.
+static Status SetNanBitsFromDictionary(KernelContext* ctx, const ArraySpan& 
arr,
+                                       uint8_t* out_bitmap, int64_t 
out_offset) {

Review Comment:
   `SetNanBitsFromDictionary` sets null bits for both NaN *and* null dictionary 
entries (via `is_null` on dictionary values). The name is misleading and also 
differs from the PR description; consider renaming it (and its call site) to 
something like `SetNullBitsFromDictionary` / `SetLogicalNullBitsFromDictionary` 
to match behavior and avoid confusion.



-- 
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]

Reply via email to