pitrou commented on code in PR #50269:
URL: https://github.com/apache/arrow/pull/50269#discussion_r3537822245
##########
cpp/src/arrow/compute/kernels/scalar_validity.cc:
##########
@@ -101,45 +80,72 @@ static void SetNanBits(const ArraySpan& arr, uint8_t*
out_bitmap, int64_t out_of
}
}
-Status IsNullExec(KernelContext* ctx, const ExecSpan& batch, ExecResult* out) {
- const ArraySpan& arr = batch[0].array;
- ArraySpan* out_span = out->array_span_mutable();
- if (arr.type->id() == Type::NA) {
- bit_util::SetBitsTo(out_span->buffers[1].data, out_span->offset,
out_span->length,
- true);
- return Status::OK();
- }
-
- const auto& options = NanOptionsState::Get(ctx);
- uint8_t* out_bitmap = out_span->buffers[1].data;
- if (arr.GetNullCount() > 0) {
- // Input has nulls => output is the inverted null (validity) bitmap.
- InvertBitmap(arr.buffers[0].data, arr.offset, arr.length, out_bitmap,
- out_span->offset);
+static Status SetLogicalNullBits(KernelContext* ctx, const ArraySpan& span,
Review Comment:
Nit: I don't think `static` is useful since this is in the anonymous
namespace already.
##########
cpp/src/arrow/util/dict_util.cc:
##########
@@ -78,5 +104,48 @@ int64_t LogicalNullCount(const ArraySpan& span) {
return LogicalNullCount<Int64Type>(span);
}
}
+
+void SetLogicalNullBits(const ArraySpan& span, uint8_t* out_bitmap, int64_t
out_offset,
+ bool set_on_null) {
+ if (span.dictionary().GetNullCount() == 0 || span.length == 0) {
Review Comment:
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?
##########
cpp/src/arrow/util/dict_util.cc:
##########
@@ -51,6 +52,31 @@ int64_t LogicalNullCount(const ArraySpan& span) {
return null_count;
}
+template <typename IndexArrowType>
+void SetLogicalNullBits(const ArraySpan& span, uint8_t* out_bitmap, int64_t
out_offset,
+ bool set_on_null) {
+ const auto* indices_null_bit_map = span.buffers[0].data;
+ const auto& dictionary_span = span.dictionary();
+ // TODO: Is this always non-null?
+ const auto* dictionary_null_bit_map = dictionary_span.buffers[0].data;
+
+ using CType = typename IndexArrowType::c_type;
+ const CType* indices_data = span.GetValues<CType>(1);
+ for (int64_t i = 0; i < span.length; i++) {
+ bool is_null = false;
+ if (indices_null_bit_map != nullptr &&
Review Comment:
Can use something like `VisitBitRuns` or `VisitNullBitmapInline`.
##########
cpp/src/arrow/util/dict_util.cc:
##########
@@ -51,6 +52,31 @@ int64_t LogicalNullCount(const ArraySpan& span) {
return null_count;
}
+template <typename IndexArrowType>
+void SetLogicalNullBits(const ArraySpan& span, uint8_t* out_bitmap, int64_t
out_offset,
+ bool set_on_null) {
+ const auto* indices_null_bit_map = span.buffers[0].data;
+ const auto& dictionary_span = span.dictionary();
+ // TODO: Is this always non-null?
+ const auto* dictionary_null_bit_map = dictionary_span.buffers[0].data;
Review Comment:
There's the special case where the dictionary values type is NA.
--
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]
