From 59ca9af32f26c178b41301433749b18711fd71ee Mon Sep 17 00:00:00 2001 From: Jeremy Schoemaker Date: Thu, 24 Sep 2026 23:27:21 -0500 Subject: [PATCH 1/4] GH-51127: [C++] Fix is_null nan_is_null for dictionary-encoded float arrays is_null(..., nan_is_null=true) did not detect NaN values in dictionary-encoded floating-point arrays. IsNullExec only checked the top-level type id for floating point, so dictionary-encoded arrays (type id DICTIONARY) skipped the NaN path entirely and NaN values in the dictionary were never marked null. Adds SetNanBitsFromDictionary, which calls is_null on the dictionary values with nan_is_null=true, maps the result through the indices with take, and ORs the resulting bitmap into the output. Hooks it into SetLogicalNullBits's DICTIONARY branch, alongside the existing dict_util::SetLogicalNullBits call that already reports null indices and null dictionary entries. Rebased onto the SetLogicalNullBits refactor that landed on main after this branch was opened. dict_util::SetLogicalNullBits now reports a null dictionary entry as a logical null unconditionally, not only under nan_is_null, so the IsNullDictionaryNullValues test's default_options expectation is updated to match (index 1 is now null even without nan_is_null). --- .../arrow/compute/kernels/scalar_validity.cc | 52 +++++++++++++++-- .../compute/kernels/scalar_validity_test.cc | 57 +++++++++++++++++++ 2 files changed, 103 insertions(+), 6 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_validity.cc b/cpp/src/arrow/compute/kernels/scalar_validity.cc index c995b8dff82f..90c4635ca7de 100644 --- a/cpp/src/arrow/compute/kernels/scalar_validity.cc +++ b/cpp/src/arrow/compute/kernels/scalar_validity.cc @@ -21,8 +21,11 @@ #include "arrow/compute/kernels/common_internal.h" #include "arrow/compute/registry_internal.h" +#include "arrow/compute/api_vector.h" +#include "arrow/type.h" #include "arrow/util/bit_util.h" #include "arrow/util/bitmap_ops.h" +#include "arrow/util/checked_cast.h" #include "arrow/util/dict_util_internal.h" #include "arrow/util/float16.h" #include "arrow/util/logging_internal.h" @@ -80,10 +83,41 @@ static void SetNanBits(const ArraySpan& arr, uint8_t* out_bitmap, int64_t out_of } } +// Maps `is_null` over the dictionary values and then through the indices, so that +// NaN dictionary entries are reported too, whatever the index type. Null dictionary +// entries and null indices are already covered by dict_util::SetLogicalNullBits, so +// this only needs to OR the NaN bits in on top of that. +static Status SetNanBitsFromDictionary(KernelContext* ctx, const ArraySpan& arr, + uint8_t* out_bitmap, int64_t out_offset) { + if (arr.length == 0) { + return Status::OK(); + } + 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(*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::BoundsCheck(), ctx->exec_context())); + + // Null and non-NaN slots are already set from dict_util::SetLogicalNullBits above, + // so the values bitmap can be OR'ed in without masking anything out of it first. + const ArrayData& result = *taken.array(); + ::arrow::internal::BitmapOr(out_bitmap, out_offset, result.buffers[1]->data(), + result.offset, arr.length, out_offset, out_bitmap); + return Status::OK(); +} + // `nan_is_null` can only be true for the is_null kernel since the is_valid and // true_unless_null kernels currently do not take `NullOptions` -Status SetLogicalNullBits(const ArraySpan& span, uint8_t* out_bitmap, int64_t out_offset, - bool set_on_null, bool nan_is_null) { +Status SetLogicalNullBits(KernelContext* ctx, const ArraySpan& span, uint8_t* out_bitmap, + int64_t out_offset, bool set_on_null, bool nan_is_null) { const Type::type t = span.type->id(); if (t == Type::NA) { // Input is all nulls, so all output bits are the same. @@ -98,8 +132,11 @@ Status SetLogicalNullBits(const ArraySpan& span, uint8_t* out_bitmap, int64_t ou // TODO: propagate `nan_is_null` ree_util::SetLogicalNullBits(span, out_bitmap, out_offset, set_on_null); } else if (t == Type::DICTIONARY) { - // TODO: propagate `nan_is_null` dict_util::SetLogicalNullBits(span, out_bitmap, out_offset, set_on_null); + const auto& dict_type = checked_cast(*span.type); + if (nan_is_null && is_floating(dict_type.value_type()->id())) { + RETURN_NOT_OK(SetNanBitsFromDictionary(ctx, span, out_bitmap, out_offset)); + } } else { // Input is a type for which logical and physical nulls are the same, so we can // use GetNullCount() and the validity bitmap @@ -141,13 +178,15 @@ Status SetLogicalNullBits(const ArraySpan& span, uint8_t* out_bitmap, int64_t ou Status IsValidExec(KernelContext* ctx, const ExecSpan& batch, ExecResult* out) { ArraySpan* out_span = out->array_span_mutable(); - return SetLogicalNullBits(batch[0].array, out_span->buffers[1].data, out_span->offset, + return SetLogicalNullBits(ctx, batch[0].array, out_span->buffers[1].data, + out_span->offset, /*set_on_null=*/false, /*nan_is_null=*/false); } Status IsNullExec(KernelContext* ctx, const ExecSpan& batch, ExecResult* out) { ArraySpan* out_span = out->array_span_mutable(); - return SetLogicalNullBits(batch[0].array, out_span->buffers[1].data, out_span->offset, + return SetLogicalNullBits(ctx, batch[0].array, out_span->buffers[1].data, + out_span->offset, /*set_on_null=*/true, /*nan_is_null=*/NanOptionsState::Get(ctx).nan_is_null); } @@ -265,7 +304,8 @@ Status TrueUnlessNullExec(KernelContext* ctx, const ExecSpan& batch, ExecResult* // NullHandling::INTERSECTION and change the validity checks in exec.cc so that // they correctly handle logical nulls, but that would invove significant changes // in exec.cc which might have more side effects - return SetLogicalNullBits(batch[0].array, out_span->buffers[0].data, out_span->offset, + return SetLogicalNullBits(ctx, batch[0].array, out_span->buffers[0].data, + out_span->offset, /*set_on_null=*/false, /*nan_is_null=*/false); } diff --git a/cpp/src/arrow/compute/kernels/scalar_validity_test.cc b/cpp/src/arrow/compute/kernels/scalar_validity_test.cc index 86895cdcec53..94ef665d1037 100644 --- a/cpp/src/arrow/compute/kernels/scalar_validity_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_validity_test.cc @@ -208,6 +208,63 @@ TEST(TestValidityKernels, IsNullSetsZeroNullCount) { ASSERT_EQ(out.array()->null_count, 0); } +TEST(TestValidityKernels, IsNullDictionaryNanIsNull) { + NullOptions default_options; + NullOptions nan_is_null_options(/*nan_is_null=*/true); + + auto dict_ty = dictionary(int32(), float64()); + auto arr = DictArrayFromJSON(dict_ty, "[0, 1, 2, null, 1]", "[1.5, NaN, -0.0]"); + + // Without nan_is_null, dictionary-encoded NaNs are not treated as null. + CheckScalarUnary("is_null", arr, + ArrayFromJSON(boolean(), "[false, false, false, true, false]")); + CheckScalarUnary("is_null", arr, + ArrayFromJSON(boolean(), "[false, false, false, true, false]"), + &default_options); + + // With nan_is_null, the dictionary entry backing index 1 is NaN, so every + // slot referencing it is null; the pre-existing null index stays null. + CheckScalarUnary("is_null", arr, + ArrayFromJSON(boolean(), "[false, true, false, true, true]"), + &nan_is_null_options); +} + +TEST(TestValidityKernels, IsNullDictionaryNullValues) { + NullOptions default_options; + NullOptions nan_is_null_options(/*nan_is_null=*/true); + + auto dict_ty = dictionary(int32(), float64()); + auto arr = DictArrayFromJSON(dict_ty, "[0, 1, 2, null]", "[1.5, null, NaN]"); + + // A null dictionary value is reported regardless of nan_is_null: index 1 points at + // a null dictionary entry, so it is a logical null even with default options. + CheckScalarUnary("is_null", arr, + ArrayFromJSON(boolean(), "[false, true, false, true]"), + &default_options); + CheckScalarUnary("is_null", arr, ArrayFromJSON(boolean(), "[false, true, true, true]"), + &nan_is_null_options); +} + +TEST(TestValidityKernels, IsNullDictionaryNanIsNullUnsignedIndices) { + NullOptions nan_is_null_options(/*nan_is_null=*/true); + + auto dict_ty = dictionary(uint8(), float32()); + auto arr = DictArrayFromJSON(dict_ty, "[2, 0, 1]", "[1.5, NaN, 2.5]"); + + CheckScalarUnary("is_null", arr, ArrayFromJSON(boolean(), "[false, false, true]"), + &nan_is_null_options); +} + +TEST(TestValidityKernels, IsNullDictionaryNanIsNullHalfFloat) { + NullOptions nan_is_null_options(/*nan_is_null=*/true); + + auto dict_ty = dictionary(int8(), float16()); + auto arr = DictArrayFromJSON(dict_ty, "[0, 1]", "[1.5, NaN]"); + + CheckScalarUnary("is_null", arr, ArrayFromJSON(boolean(), "[false, true]"), + &nan_is_null_options); +} + template class TestFloatingPointValidityKernels : public TestValidityKernels { public: From 61bef91914734dfb0a1d5058b6b31d45801cb7ff Mon Sep 17 00:00:00 2001 From: Jeremy Schoemaker Date: Thu, 24 Sep 2026 23:59:36 -0500 Subject: [PATCH 2/4] GH-51127: [C++] Check dictionary indices before validity lookup --- .../arrow/compute/kernels/scalar_validity.cc | 26 ++++++++------- .../compute/kernels/scalar_validity_test.cc | 32 +++++++++++++++++-- 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_validity.cc b/cpp/src/arrow/compute/kernels/scalar_validity.cc index 90c4635ca7de..b13669bd7639 100644 --- a/cpp/src/arrow/compute/kernels/scalar_validity.cc +++ b/cpp/src/arrow/compute/kernels/scalar_validity.cc @@ -83,20 +83,22 @@ static void SetNanBits(const ArraySpan& arr, uint8_t* out_bitmap, int64_t out_of } } -// Maps `is_null` over the dictionary values and then through the indices, so that -// NaN dictionary entries are reported too, whatever the index type. Null dictionary -// entries and null indices are already covered by dict_util::SetLogicalNullBits, so -// this only needs to OR the NaN bits in on top of that. +// 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) { 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())); + 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(*arr.type); auto indices = ArrayData::Make(dict_type.index_type(), arr.length, @@ -106,8 +108,8 @@ static Status SetNanBitsFromDictionary(KernelContext* ctx, const ArraySpan& arr, Take(dict_is_null, Datum(std::move(indices)), TakeOptions::BoundsCheck(), ctx->exec_context())); - // Null and non-NaN slots are already set from dict_util::SetLogicalNullBits above, - // so the values bitmap can be OR'ed in without masking anything out of it first. + // 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(), result.offset, arr.length, out_offset, out_bitmap); @@ -132,11 +134,11 @@ Status SetLogicalNullBits(KernelContext* ctx, const ArraySpan& span, uint8_t* ou // TODO: propagate `nan_is_null` ree_util::SetLogicalNullBits(span, out_bitmap, out_offset, set_on_null); } else if (t == Type::DICTIONARY) { - dict_util::SetLogicalNullBits(span, out_bitmap, out_offset, set_on_null); const auto& dict_type = checked_cast(*span.type); if (nan_is_null && is_floating(dict_type.value_type()->id())) { - RETURN_NOT_OK(SetNanBitsFromDictionary(ctx, span, out_bitmap, out_offset)); + return SetNanBitsFromDictionary(ctx, span, out_bitmap, out_offset); } + dict_util::SetLogicalNullBits(span, out_bitmap, out_offset, set_on_null); } else { // Input is a type for which logical and physical nulls are the same, so we can // use GetNullCount() and the validity bitmap diff --git a/cpp/src/arrow/compute/kernels/scalar_validity_test.cc b/cpp/src/arrow/compute/kernels/scalar_validity_test.cc index 94ef665d1037..6d38913047dd 100644 --- a/cpp/src/arrow/compute/kernels/scalar_validity_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_validity_test.cc @@ -238,8 +238,7 @@ TEST(TestValidityKernels, IsNullDictionaryNullValues) { // A null dictionary value is reported regardless of nan_is_null: index 1 points at // a null dictionary entry, so it is a logical null even with default options. - CheckScalarUnary("is_null", arr, - ArrayFromJSON(boolean(), "[false, true, false, true]"), + CheckScalarUnary("is_null", arr, ArrayFromJSON(boolean(), "[false, true, false, true]"), &default_options); CheckScalarUnary("is_null", arr, ArrayFromJSON(boolean(), "[false, true, true, true]"), &nan_is_null_options); @@ -255,6 +254,35 @@ TEST(TestValidityKernels, IsNullDictionaryNanIsNullUnsignedIndices) { &nan_is_null_options); } +TEST(TestValidityKernels, IsNullDictionaryNanIsNullBounds) { + NullOptions options(/*nan_is_null=*/true); + auto dict_ty = dictionary(int32(), float64()); + auto values = ArrayFromJSON(float64(), "[1.5, null, NaN]"); + for (const auto* indices_json : + {"[0, -1]", "[0, 3]", "[0, -2147483648]", "[0, 2147483647]"}) { + SCOPED_TRACE(indices_json); + auto indices = ArrayFromJSON(int32(), indices_json); + auto arr = std::make_shared(dict_ty, indices, values); + ASSERT_RAISES(IndexError, IsNull(arr, options)); + } + + auto empty = DictArrayFromJSON(dict_ty, "[]", "[]"); + CheckScalarUnary("is_null", empty, ArrayFromJSON(boolean(), "[]"), &options); +} + +TEST(TestValidityKernels, IsNullDictionaryNanIsNullSkipsNullIndex) { + NullOptions options(/*nan_is_null=*/true); + auto dict_ty = dictionary(int32(), float64()); + auto indices = ArrayFromJSON(int32(), "[0, null, 1]"); + // The physical value in a null slot must not be dereferenced. + reinterpret_cast(indices->data()->buffers[1]->mutable_data())[1] = -1; + auto values = ArrayFromJSON(float64(), "[1.5, NaN]"); + auto arr = std::make_shared(dict_ty, indices, values); + ASSERT_OK(arr->ValidateFull()); + CheckScalarUnary("is_null", arr, ArrayFromJSON(boolean(), "[false, true, true]"), + &options); +} + TEST(TestValidityKernels, IsNullDictionaryNanIsNullHalfFloat) { NullOptions nan_is_null_options(/*nan_is_null=*/true); From ca9631491b286bc72b4c7504f1832df0f47d5ef5 Mon Sep 17 00:00:00 2001 From: Jeremy Schoemaker Date: Mon, 28 Sep 2026 05:03:29 -0500 Subject: [PATCH 3/4] GH-51127: [C++] Simplify dictionary NaN null checks --- .../arrow/compute/kernels/scalar_validity.cc | 4 +- .../compute/kernels/scalar_validity_test.cc | 94 ++++--------------- 2 files changed, 20 insertions(+), 78 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_validity.cc b/cpp/src/arrow/compute/kernels/scalar_validity.cc index b13669bd7639..517cbc7cff15 100644 --- a/cpp/src/arrow/compute/kernels/scalar_validity.cc +++ b/cpp/src/arrow/compute/kernels/scalar_validity.cc @@ -83,7 +83,7 @@ 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 +// 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) { @@ -106,7 +106,7 @@ static Status SetNanBitsFromDictionary(KernelContext* ctx, const ArraySpan& arr, arr.offset); ARROW_ASSIGN_OR_RAISE(Datum taken, Take(dict_is_null, Datum(std::move(indices)), - TakeOptions::BoundsCheck(), ctx->exec_context())); + 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. diff --git a/cpp/src/arrow/compute/kernels/scalar_validity_test.cc b/cpp/src/arrow/compute/kernels/scalar_validity_test.cc index 6d38913047dd..f3b607b6bb2e 100644 --- a/cpp/src/arrow/compute/kernels/scalar_validity_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_validity_test.cc @@ -212,85 +212,27 @@ TEST(TestValidityKernels, IsNullDictionaryNanIsNull) { NullOptions default_options; NullOptions nan_is_null_options(/*nan_is_null=*/true); - auto dict_ty = dictionary(int32(), float64()); - auto arr = DictArrayFromJSON(dict_ty, "[0, 1, 2, null, 1]", "[1.5, NaN, -0.0]"); - - // Without nan_is_null, dictionary-encoded NaNs are not treated as null. - CheckScalarUnary("is_null", arr, - ArrayFromJSON(boolean(), "[false, false, false, true, false]")); - CheckScalarUnary("is_null", arr, - ArrayFromJSON(boolean(), "[false, false, false, true, false]"), - &default_options); - - // With nan_is_null, the dictionary entry backing index 1 is NaN, so every - // slot referencing it is null; the pre-existing null index stays null. - CheckScalarUnary("is_null", arr, - ArrayFromJSON(boolean(), "[false, true, false, true, true]"), - &nan_is_null_options); -} - -TEST(TestValidityKernels, IsNullDictionaryNullValues) { - NullOptions default_options; - NullOptions nan_is_null_options(/*nan_is_null=*/true); - - auto dict_ty = dictionary(int32(), float64()); - auto arr = DictArrayFromJSON(dict_ty, "[0, 1, 2, null]", "[1.5, null, NaN]"); - - // A null dictionary value is reported regardless of nan_is_null: index 1 points at - // a null dictionary entry, so it is a logical null even with default options. - CheckScalarUnary("is_null", arr, ArrayFromJSON(boolean(), "[false, true, false, true]"), - &default_options); - CheckScalarUnary("is_null", arr, ArrayFromJSON(boolean(), "[false, true, true, true]"), - &nan_is_null_options); -} + for (const auto& value_type : {float16(), float32(), float64()}) { + SCOPED_TRACE(value_type->ToString()); + auto dict_ty = dictionary(int32(), value_type); + auto arr = + DictArrayFromJSON(dict_ty, "[0, 1, 2, 3, null, 1]", "[1.5, NaN, -0.0, null]"); -TEST(TestValidityKernels, IsNullDictionaryNanIsNullUnsignedIndices) { - NullOptions nan_is_null_options(/*nan_is_null=*/true); - - auto dict_ty = dictionary(uint8(), float32()); - auto arr = DictArrayFromJSON(dict_ty, "[2, 0, 1]", "[1.5, NaN, 2.5]"); - - CheckScalarUnary("is_null", arr, ArrayFromJSON(boolean(), "[false, false, true]"), - &nan_is_null_options); -} + // Null dictionary values and null indices are always null. + CheckScalarUnary( + "is_null", arr, + ArrayFromJSON(boolean(), "[false, false, false, true, true, false]")); + CheckScalarUnary("is_null", arr, + ArrayFromJSON(boolean(), "[false, false, false, true, true, false]"), + &default_options); + CheckScalarUnary("is_null", arr, + ArrayFromJSON(boolean(), "[false, true, false, true, true, true]"), + &nan_is_null_options); -TEST(TestValidityKernels, IsNullDictionaryNanIsNullBounds) { - NullOptions options(/*nan_is_null=*/true); - auto dict_ty = dictionary(int32(), float64()); - auto values = ArrayFromJSON(float64(), "[1.5, null, NaN]"); - for (const auto* indices_json : - {"[0, -1]", "[0, 3]", "[0, -2147483648]", "[0, 2147483647]"}) { - SCOPED_TRACE(indices_json); - auto indices = ArrayFromJSON(int32(), indices_json); - auto arr = std::make_shared(dict_ty, indices, values); - ASSERT_RAISES(IndexError, IsNull(arr, options)); + auto empty = DictArrayFromJSON(dict_ty, "[]", "[]"); + CheckScalarUnary("is_null", empty, ArrayFromJSON(boolean(), "[]"), + &nan_is_null_options); } - - auto empty = DictArrayFromJSON(dict_ty, "[]", "[]"); - CheckScalarUnary("is_null", empty, ArrayFromJSON(boolean(), "[]"), &options); -} - -TEST(TestValidityKernels, IsNullDictionaryNanIsNullSkipsNullIndex) { - NullOptions options(/*nan_is_null=*/true); - auto dict_ty = dictionary(int32(), float64()); - auto indices = ArrayFromJSON(int32(), "[0, null, 1]"); - // The physical value in a null slot must not be dereferenced. - reinterpret_cast(indices->data()->buffers[1]->mutable_data())[1] = -1; - auto values = ArrayFromJSON(float64(), "[1.5, NaN]"); - auto arr = std::make_shared(dict_ty, indices, values); - ASSERT_OK(arr->ValidateFull()); - CheckScalarUnary("is_null", arr, ArrayFromJSON(boolean(), "[false, true, true]"), - &options); -} - -TEST(TestValidityKernels, IsNullDictionaryNanIsNullHalfFloat) { - NullOptions nan_is_null_options(/*nan_is_null=*/true); - - auto dict_ty = dictionary(int8(), float16()); - auto arr = DictArrayFromJSON(dict_ty, "[0, 1]", "[1.5, NaN]"); - - CheckScalarUnary("is_null", arr, ArrayFromJSON(boolean(), "[false, true]"), - &nan_is_null_options); } template From a78401dcaede58ccfc0d982cc64153d67faed2f9 Mon Sep 17 00:00:00 2001 From: Jeremy Schoemaker Date: Mon, 28 Sep 2026 08:35:24 -0500 Subject: [PATCH 4/4] GH-51127: Combine dictionary null bits in one bitmap pass --- .../arrow/compute/kernels/scalar_validity.cc | 17 ++++++++--------- .../compute/kernels/scalar_validity_test.cc | 6 ++++++ 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_validity.cc b/cpp/src/arrow/compute/kernels/scalar_validity.cc index 517cbc7cff15..0829af59dc1f 100644 --- a/cpp/src/arrow/compute/kernels/scalar_validity.cc +++ b/cpp/src/arrow/compute/kernels/scalar_validity.cc @@ -90,11 +90,6 @@ static Status SetNanBitsFromDictionary(KernelContext* ctx, const ArraySpan& arr, 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()}, @@ -108,11 +103,15 @@ static Status SetNanBitsFromDictionary(KernelContext* ctx, const ArraySpan& arr, 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(), - result.offset, arr.length, out_offset, out_bitmap); + if (arr.GetNullCount() > 0) { + ::arrow::internal::BitmapOrNot(result.buffers[1]->data(), result.offset, + arr.buffers[0].data, arr.offset, arr.length, + out_offset, out_bitmap); + } else { + CopyBitmap(result.buffers[1]->data(), result.offset, arr.length, out_bitmap, + out_offset); + } return Status::OK(); } diff --git a/cpp/src/arrow/compute/kernels/scalar_validity_test.cc b/cpp/src/arrow/compute/kernels/scalar_validity_test.cc index f3b607b6bb2e..0a041e7e47f7 100644 --- a/cpp/src/arrow/compute/kernels/scalar_validity_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_validity_test.cc @@ -229,6 +229,12 @@ TEST(TestValidityKernels, IsNullDictionaryNanIsNull) { ArrayFromJSON(boolean(), "[false, true, false, true, true, true]"), &nan_is_null_options); + auto no_null_indices = + DictArrayFromJSON(dict_ty, "[3, 1, 0, 2]", "[1.5, NaN, -0.0, null]"); + CheckScalarUnary("is_null", no_null_indices, + ArrayFromJSON(boolean(), "[true, true, false, false]"), + &nan_is_null_options); + auto empty = DictArrayFromJSON(dict_ty, "[]", "[]"); CheckScalarUnary("is_null", empty, ArrayFromJSON(boolean(), "[]"), &nan_is_null_options);