ARROW-36420

R-JunmingChen · Aug 18, 2023 · 5be997c · 5be997c
1 parent b79a4dc
commit 5be997c
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 10 deletions.
diff --git a/cpp/src/arrow/compute/api_scalar.cc b/cpp/src/arrow/compute/api_scalar.cc
@@ -275,6 +275,29 @@ struct EnumTraits<compute::MapLookupOptions::Occurrence>
   }
 };
 
+template <>
+struct EnumTraits<compute::SetLookupOptions::NullMatchingBehavior>
+    : BasicEnumTraits<compute::SetLookupOptions::NullMatchingBehavior,
+                      compute::SetLookupOptions::NullMatchingBehavior::MATCH,
+                      compute::SetLookupOptions::NullMatchingBehavior::SKIP,
+                      compute::SetLookupOptions::NullMatchingBehavior::EMIT_NULL,
+                     compute::SetLookupOptions::NullMatchingBehavior::INCONCLUSIVE> {
+  static std::string name() { return "SetLookupOptions::NullMatchingBehavior"; }
+  static std::string value_name(compute::SetLookupOptions::NullMatchingBehavior value) {
+    switch (value) {
+      case compute::SetLookupOptions::NullMatchingBehavior::MATCH:
+        return "MATCH";
+      case compute::SetLookupOptions::NullMatchingBehavior::SKIP:
+        return "SKIP";
+      case compute::SetLookupOptions::NullMatchingBehavior::EMIT_NULL:
+        return "EMIT_NULL";
+      case compute::SetLookupOptions::NullMatchingBehavior::INCONCLUSIVE:
+        return "INCONCLUSIVE";
+    }
+    return "<INVALID>";
+  }
+};
+
 }  // namespace internal
 
 namespace compute {
@@ -555,13 +578,13 @@ SetLookupOptions::SetLookupOptions(
       null_matching_behavior(std::move(null_matching_behavior)) {}
 SetLookupOptions::SetLookupOptions()
     : SetLookupOptions({}, SetLookupOptions::NullMatchingBehavior::MATCH) {}
-SetLookupOptions::NullMatchingBehavior SetLookupOptions::getNullMatchingBehavior() {
+SetLookupOptions::NullMatchingBehavior SetLookupOptions::getNullMatchingBehavior() const {
   if (this->skip_nulls == std::nullopt) {
     return this->null_matching_behavior;
   } else if (this->skip_nulls) {
-    return SetLookupOptions::NullMatchingBehavior::SKIP;
+    return SetLookupOptions::SKIP;
   } else {
-    return SetLookupOptions::NullMatchingBehavior::MATCH;
+    return SetLookupOptions::MATCH;
   }
 }
 constexpr char SetLookupOptions::kTypeName[];

diff --git a/cpp/src/arrow/compute/api_scalar.h b/cpp/src/arrow/compute/api_scalar.h
@@ -271,17 +271,18 @@ class ARROW_EXPORT SetLookupOptions : public FunctionOptions {
   enum NullMatchingBehavior { MATCH, SKIP, EMIT_NULL, INCONCLUSIVE };
 
   explicit SetLookupOptions(Datum value_set, NullMatchingBehavior = MATCH);
-  explicit SetLookupOptions(Datum value_set, bool skip_nulls);  SetLookupOptions();
+  explicit SetLookupOptions(Datum value_set, bool skip_nulls);
+  SetLookupOptions();
   static constexpr char const kTypeName[] = "SetLookupOptions";
 
   /// The set of values to look up input values into.
   Datum value_set;
 
   NullMatchingBehavior null_matching_behavior;
-  
+
   // DEPRECATED(will be removed after removing of skip_nulls)
-   NullMatchingBehavior const getNullMatchingBehavior();
-  
+  NullMatchingBehavior getNullMatchingBehavior() const;
+
   // DEPRECATED(use null_matching_behavior instead)
   /// Whether nulls in `value_set` count for lookup.
   ///

diff --git a/cpp/src/arrow/compute/kernels/scalar_set_lookup.cc b/cpp/src/arrow/compute/kernels/scalar_set_lookup.cc
@@ -280,10 +280,12 @@ struct IndexInVisitor {
     const auto& state = checked_cast<const SetLookupState<NullType>&>(*ctx->state());
 
     if (data.length != 0) {
-      // skip_nulls is honored for consistency with other types
-      bit_util::SetBitsTo(out_bitmap, out->offset, out->length, state.value_set_has_null);
+      bit_util::SetBitsTo(out_bitmap, out->offset, out->length,
+                          state.null_matching_behavior == SetLookupOptions::MATCH &&
+                              state.value_set_has_null);
 
       // Set all values to 0, which will be unmasked only if null is in the value_set
+      // and null_matching_behavior is equal to MATCH
       std::memset(out->GetValues<int32_t>(1), 0x00, out->length * sizeof(int32_t));
     }
     return Status::OK();
@@ -311,7 +313,8 @@ struct IndexInVisitor {
           bitmap_writer.Next();
         },
         [&]() {
-          if (state.null_index != -1) {
+          if (state.null_index != -1 &&
+              state.null_matching_behavior == SetLookupOptions::MATCH) {
             bitmap_writer.Set();
 
             // value_set included null