GH-46183: [C++][Compute] Make is_in and index_in treat -0.0 and +0.0 as equal

cpp/src/arrow/compute/expression_test.cc

@@ -2178,6 +2178,20 @@ TEST(Expression, SimplifyThenExecute) {
   AssertDatumsEqual(evaluated, simplified_evaluated, /*verbose=*/true);
 }
 
+TEST(Expression, SimplifyIsInSignedZero) {
+  // Parquet statistics of a column holding only zeros are min=-0.0 and max=+0.0, which
+  // become the guarantee `x == -0.0`. It must not rule out `x in {0.0}`.
+  SetLookupOptions in_zero_f64{ArrayFromJSON(float64(), "[0.0]")};
+  Simplify{call("is_in", {field_ref("f64")}, in_zero_f64)}
+      .WithGuarantee(equal(field_ref("f64"), literal(-0.0)))
+      .Expect(true);
+
+  SetLookupOptions in_zero_f32{ArrayFromJSON(float32(), "[0.0]")};
+  Simplify{call("is_in", {field_ref("f32")}, in_zero_f32)}
+      .WithGuarantee(equal(field_ref("f32"), literal(-0.0f)))
+      .Expect(true);
+}
+
 TEST(Expression, SimplifyIsInThenExecute) {
   auto input = RecordBatchFromJSON(kBoringSchema, R"([
       {"i64": 2, "i32": 5},

cpp/src/arrow/compute/kernels/scalar_set_lookup.cc

@@ -15,13 +15,16 @@
 // specific language governing permissions and limitations
 // under the License.
 
+#include <type_traits>
+
 #include "arrow/array/array_base.h"
 #include "arrow/compute/api_scalar.h"
 #include "arrow/compute/cast.h"
 #include "arrow/compute/kernels/common_internal.h"
 #include "arrow/compute/kernels/util_internal.h"
 #include "arrow/compute/registry_internal.h"
 #include "arrow/type.h"
+#include "arrow/type_traits.h"
 #include "arrow/util/bit_util.h"
 #include "arrow/util/bitmap_writer.h"
 #include "arrow/util/hashing.h"
@@ -47,6 +50,7 @@ struct SetLookupState : public SetLookupStateBase {
 
   Status Init(const SetLookupOptions& options) {
     this->null_matching_behavior = options.GetNullMatchingBehavior();
+    this->value_set_is_floating = is_floating(options.value_set.type()->id());
     if (options.value_set.is_array()) {
       const ArrayData& value_set = *options.value_set.array();
       memo_index_to_value_index.reserve(value_set.length);
@@ -92,8 +96,9 @@ struct SetLookupState : public SetLookupStateBase {
         DCHECK_EQ(memo_index, memo_size);
         memo_index_to_value_index.push_back(index);
       };
-      RETURN_NOT_OK(lookup_table->GetOrInsert(
-          v, std::move(on_found), std::move(on_not_found), &unused_memo_index));
+      RETURN_NOT_OK(lookup_table->GetOrInsert(LookupKey(v), std::move(on_found),
+                                              std::move(on_not_found),
+                                              &unused_memo_index));
       ++index;
       return Status::OK();
     };
@@ -114,9 +119,22 @@ struct SetLookupState : public SetLookupStateBase {
     return VisitArraySpanInline<Type>(data, visit_valid, visit_null);
   }
 
+  // Floating-point values are looked up through their unsigned integer bit patterns
+  // (see InitStateVisitor), which would make -0.0 and +0.0 different keys. Fold -0.0
+  // into +0.0 so that set membership agrees with `equal`.
+  template <typename T>
+  T LookupKey(T value) const {
+    if constexpr (std::is_integral_v<T> && !std::is_same_v<T, bool>) {
+      constexpr T kNegativeZero = static_cast<T>(T{1} << (sizeof(T) * 8 - 1));
+      if (value == kNegativeZero && value_set_is_floating) return T{0};
+    }
+    return value;
+  }
+
   using MemoTable = typename HashTraits<Type>::MemoTableType;
   std::optional<MemoTable> lookup_table;  // use optional for delayed initialization
   MemoryPool* memory_pool;
+  bool value_set_is_floating = false;
   // When there are duplicates in value_set, the MemoTable indices must
   // be mapped back to indices in the value_set.
   std::vector<int32_t> memo_index_to_value_index;
@@ -305,7 +323,7 @@ struct IndexInVisitor {
     VisitArraySpanInline<Type>(
         input,
         [&](T v) {
-          int32_t index = state.lookup_table->Get(v);
+          int32_t index = state.lookup_table->Get(state.LookupKey(v));
           if (index != -1) {
             bitmap_writer.Set();
 
@@ -441,7 +459,7 @@ struct IsInVisitor {
     VisitArraySpanInline<Type>(
         input,
         [&](T v) {
-          if (state.lookup_table->Get(v) != -1) {  // true
+          if (state.lookup_table->Get(state.LookupKey(v)) != -1) {  // true
             writer_boolean.Set();
             writer_null.Set();
           } else if (state.null_matching_behavior == SetLookupOptions::INCONCLUSIVE &&

cpp/src/arrow/compute/kernels/scalar_set_lookup_test.cc

@@ -20,6 +20,7 @@
 #include <cstdio>
 #include <functional>
 #include <iosfwd>
+#include <limits>
 #include <locale>
 #include <memory>
 #include <stdexcept>
@@ -36,6 +37,7 @@
 #include "arrow/compute/kernels/test_util_internal.h"
 #include "arrow/result.h"
 #include "arrow/status.h"
+#include "arrow/testing/builder.h"
 #include "arrow/testing/gtest_compat.h"
 #include "arrow/testing/gtest_util.h"
 #include "arrow/type.h"
@@ -305,6 +307,48 @@ TYPED_TEST(TestIsInKernelPrimitive, IsIn) {
   CheckIsIn(type, "[]", "[]", "[]");
 }
 
+// -0.0 and +0.0 compare equal, so each of them must be found in a set holding the other
+template <typename ArrowType>
+void CheckIsInSignedZero() {
+  using CType = typename ArrowType::c_type;
+  std::shared_ptr<Array> input, positive_zero, negative_zero, both;
+  ArrayFromVector<ArrowType>({CType(-0.0), CType(0.0), CType(1.5)}, &input);
+  ArrayFromVector<ArrowType>({CType(0.0)}, &positive_zero);
+  ArrayFromVector<ArrowType>({CType(-0.0)}, &negative_zero);
+  ArrayFromVector<ArrowType>({CType(2.5), CType(-0.0), CType(0.0)}, &both);
+
+  CheckIsIn(input, positive_zero, "[true, true, false]");
+  CheckIsIn(input, negative_zero, "[true, true, false]");
+  CheckIsIn(input, both, "[true, true, false]");
+
+  std::shared_ptr<Array> other;
+  ArrayFromVector<ArrowType>({CType(2.5)}, &other);
+  CheckIsInChunked(std::make_shared<ChunkedArray>(ArrayVector{input}),
+                   std::make_shared<ChunkedArray>(ArrayVector{other, positive_zero}),
+                   ChunkedArrayFromJSON(boolean(), {"[true, true, false]"}));
+}
+
+// The bit pattern of -0.0 is the smallest signed integer of the same width,
+// which must not be folded into 0
+template <typename ArrowType>
+void CheckIsInSmallestInteger() {
+  using CType = typename ArrowType::c_type;
+  std::shared_ptr<Array> input, zero, smallest;
+  ArrayFromVector<ArrowType>({std::numeric_limits<CType>::min(), CType(0)}, &input);
+  ArrayFromVector<ArrowType>({CType(0)}, &zero);
+  ArrayFromVector<ArrowType>({std::numeric_limits<CType>::min()}, &smallest);
+
+  CheckIsIn(input, zero, "[false, true]");
+  CheckIsIn(input, smallest, "[true, false]");
+}
+
+TEST_F(TestIsInKernel, SignedZero) {
+  CheckIsInSignedZero<FloatType>();
+  CheckIsInSignedZero<DoubleType>();
+  CheckIsInSmallestInteger<Int32Type>();
+  CheckIsInSmallestInteger<Int64Type>();
+}
+
 TEST_F(TestIsInKernel, NullType) {
   auto type = null();
 
@@ -1430,6 +1474,40 @@ TEST_F(TestIndexInKernel, Boolean) {
   CheckIndexIn(boolean(), "[null, null, null, null]", "[null]", "[0, 0, 0, 0]");
 }
 
+template <typename ArrowType>
+void CheckIndexInSignedZero(TestIndexInKernel* test) {
+  using CType = typename ArrowType::c_type;
+  std::shared_ptr<Array> input, positive_zero, negative_zero, both;
+  ArrayFromVector<ArrowType>({CType(-0.0), CType(0.0), CType(1.5)}, &input);
+  ArrayFromVector<ArrowType>({CType(0.0)}, &positive_zero);
+  ArrayFromVector<ArrowType>({CType(-0.0)}, &negative_zero);
+  ArrayFromVector<ArrowType>({CType(2.5), CType(-0.0), CType(0.0)}, &both);
+
+  test->CheckIndexIn(input, positive_zero, "[0, 0, null]");
+  test->CheckIndexIn(input, negative_zero, "[0, 0, null]");
+  // Both zeros are the same value: the first one in the value set is reported
+  test->CheckIndexIn(input, both, "[1, 1, null]");
+}
+
+template <typename ArrowType>
+void CheckIndexInSmallestInteger(TestIndexInKernel* test) {
+  using CType = typename ArrowType::c_type;
+  std::shared_ptr<Array> input, zero, smallest;
+  ArrayFromVector<ArrowType>({std::numeric_limits<CType>::min(), CType(0)}, &input);
+  ArrayFromVector<ArrowType>({CType(0)}, &zero);
+  ArrayFromVector<ArrowType>({std::numeric_limits<CType>::min()}, &smallest);
+
+  test->CheckIndexIn(input, zero, "[null, 0]");
+  test->CheckIndexIn(input, smallest, "[0, null]");
+}
+
+TEST_F(TestIndexInKernel, SignedZero) {
+  CheckIndexInSignedZero<FloatType>(this);
+  CheckIndexInSignedZero<DoubleType>(this);
+  CheckIndexInSmallestInteger<Int32Type>(this);
+  CheckIndexInSmallestInteger<Int64Type>(this);
+}
+
 TEST_F(TestIndexInKernel, ImplicitlyCastValueSet) {
   auto input = ArrayFromJSON(int8(), "[0, 1, 2, 3, 4, 5, 6, 7, 8]");
 

python/pyarrow/tests/test_compute.py

@@ -3462,6 +3462,15 @@ def test_index_in():
     assert result.to_pylist() == [0, None, None, 0, None, 1]
 
 
+@pytest.mark.parametrize("typ", [pa.float32(), pa.float64()])
+def test_is_in_index_in_signed_zero(typ):
+    # -0.0 == 0.0, so membership must not depend on the sign of the zero
+    arr = pa.array([-0.0, 0.0, 1.5], type=typ)
+    for value_set in (pa.array([0.0], type=typ), pa.array([-0.0], type=typ)):
+        assert pc.is_in(arr, value_set=value_set).to_pylist() == [True, True, False]
+        assert pc.index_in(arr, value_set=value_set).to_pylist() == [0, 0, None]
+
+
 def test_quantile():
     arr = pa.array([1, 2, 3, 4])
 

python/pyarrow/tests/test_dataset.py

@@ -3155,6 +3155,18 @@ def test_filter_equal_null(tempdir, dataset_reader):
     assert table.num_rows == 0
 
 
+@pytest.mark.parquet
+def test_filter_isin_signed_zero(tempdir, dataset_reader):
+    # A column holding only zeros has Parquet statistics min=-0.0 and max=0.0,
+    # which must not make the fragment look like it cannot contain 0.0
+    table = pa.table({"x": [0.0, 0.0]})
+    _, path = _create_single_file(tempdir, table)
+    dataset = ds.dataset(str(path))
+
+    filter_ = ds.field("x").isin([0.0])
+    assert dataset_reader.to_table(dataset, filter=filter_).equals(table)
+
+
 @pytest.mark.parquet
 def test_filter_compute_expression(tempdir, dataset_reader):
     table = pa.table({