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


##########
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 the 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) {
+  if (arr.length == 0) {
+    return Status::OK();
+  }
+  if (arr.GetNullCount() > 0) {
+    InvertBitmap(arr.buffers[0].data, arr.offset, arr.length, out_bitmap, 
out_offset);
+  } else {
+    bit_util::SetBitsTo(out_bitmap, out_offset, arr.length, false);
+  }
+  NullOptions nan_is_null_options(/*nan_is_null=*/true);
+  ARROW_ASSIGN_OR_RAISE(Datum dict_is_null,
+                        CallFunction("is_null", 
{arr.dictionary().ToArrayData()},
+                                     &nan_is_null_options, 
ctx->exec_context()));
+
+  const auto& dict_type = checked_cast<const DictionaryType&>(*arr.type);
+  auto indices = ArrayData::Make(dict_type.index_type(), arr.length,
+                                 {arr.GetBuffer(0), arr.GetBuffer(1)}, 
arr.GetNullCount(),
+                                 arr.offset);
+  ARROW_ASSIGN_OR_RAISE(Datum taken,
+                        Take(dict_is_null, Datum(std::move(indices)),
+                             TakeOptions::NoBoundsCheck(), 
ctx->exec_context()));
+
+  // Null index slots are already set from the input validity bitmap, so the
+  // values bitmap can be OR'ed in without masking null slots out first.
+  const ArrayData& result = *taken.array();
+  ::arrow::internal::BitmapOr(out_bitmap, out_offset, 
result.buffers[1]->data(),

Review Comment:
   Changed in `a78401dca`: `BitmapOrNot` combines the taken bits with inverted 
index validity in one pass; arrays without null indices use `CopyBitmap`. The 
float16/32/64 test now also covers non-null indices pointing to null/NaN 
values. Rebuilt scalar-utility suite: 97 tests passed (macOS arm64 Debug); 
clang-format 18.1.8 and cpplint 1.6.1 passed. AI-assisted implementation and 
validation.



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