From e0706121e0554f3f64403186078e2ad20e0be71d Mon Sep 17 00:00:00 2001 From: liyafan82 Date: Fri, 24 Jul 2020 17:38:44 +0800 Subject: [PATCH] ARROW-9495: [C++] Equality assertions don't handle Inf / -Inf properly --- cpp/src/arrow/array/array_test.cc | 74 ++++++++++++++++++++++++++++++- cpp/src/arrow/compare.cc | 7 +-- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc index 2702c355c01..7af1a3d3bac 100644 --- a/cpp/src/arrow/array/array_test.cc +++ b/cpp/src/arrow/array/array_test.cc @@ -1501,7 +1501,7 @@ void CheckFloatingNanEquality() { // NaN != non-NaN ArrayFromVector(type, {false, true}, {0.5, nan_value}, &a); - ArrayFromVector(type, {false, true}, {0.5, 0.0}, &a); + ArrayFromVector(type, {false, true}, {0.5, 0.0}, &b); ASSERT_FALSE(a->Equals(b)); ASSERT_FALSE(b->Equals(a)); ASSERT_FALSE(a->Equals(b, EqualOptions().nans_equal(true))); @@ -1520,6 +1520,73 @@ void CheckFloatingNanEquality() { ASSERT_TRUE(b->RangeEquals(a, 0, 1, 0)); } +template +void CheckFloatingInfinityEquality() { + std::shared_ptr a, b; + std::shared_ptr type = TypeTraits::type_singleton(); + + const auto infinity = std::numeric_limits::infinity(); + + for (auto nans_equal : {false, true}) { + // Infinity in a null entry + ArrayFromVector(type, {true, false}, {0.5, infinity}, &a); + ArrayFromVector(type, {true, false}, {0.5, -infinity}, &b); + ASSERT_TRUE(a->Equals(b)); + ASSERT_TRUE(b->Equals(a)); + ASSERT_TRUE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); + ASSERT_TRUE(b->ApproxEquals(a, EqualOptions().atol(1e-5).nans_equal(nans_equal))); + ASSERT_TRUE(a->RangeEquals(b, 0, 2, 0)); + ASSERT_TRUE(b->RangeEquals(a, 0, 2, 0)); + ASSERT_TRUE(a->RangeEquals(b, 1, 2, 1)); + ASSERT_TRUE(b->RangeEquals(a, 1, 2, 1)); + + // Infinity in a valid entry + ArrayFromVector(type, {false, true}, {0.5, infinity}, &a); + ArrayFromVector(type, {false, true}, {0.5, infinity}, &b); + ASSERT_TRUE(a->Equals(b)); + ASSERT_TRUE(b->Equals(a)); + ASSERT_TRUE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); + ASSERT_TRUE(b->ApproxEquals(a, EqualOptions().atol(1e-5).nans_equal(nans_equal))); + ASSERT_TRUE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); + ASSERT_TRUE(b->ApproxEquals(a, EqualOptions().atol(1e-5).nans_equal(nans_equal))); + // Infinity in tested range + ASSERT_TRUE(a->RangeEquals(b, 0, 2, 0)); + ASSERT_TRUE(b->RangeEquals(a, 0, 2, 0)); + ASSERT_TRUE(a->RangeEquals(b, 1, 2, 1)); + ASSERT_TRUE(b->RangeEquals(a, 1, 2, 1)); + // Infinity not in tested range + ASSERT_TRUE(a->RangeEquals(b, 0, 1, 0)); + ASSERT_TRUE(b->RangeEquals(a, 0, 1, 0)); + + // Infinity != non-infinity + ArrayFromVector(type, {false, true}, {0.5, -infinity}, &a); + ArrayFromVector(type, {false, true}, {0.5, 0.0}, &b); + ASSERT_FALSE(a->Equals(b)); + ASSERT_FALSE(b->Equals(a)); + ASSERT_FALSE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); + ASSERT_FALSE(b->ApproxEquals(a)); + ASSERT_FALSE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); + ASSERT_FALSE(b->ApproxEquals(a, EqualOptions().atol(1e-5).nans_equal(nans_equal))); + // Infinity != Negative infinity + ArrayFromVector(type, {true, true}, {0.5, -infinity}, &a); + ArrayFromVector(type, {true, true}, {0.5, infinity}, &b); + ASSERT_FALSE(a->Equals(b)); + ASSERT_FALSE(b->Equals(a)); + ASSERT_FALSE(a->ApproxEquals(b)); + ASSERT_FALSE(b->ApproxEquals(a)); + ASSERT_FALSE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); + ASSERT_FALSE(b->ApproxEquals(a, EqualOptions().atol(1e-5).nans_equal(nans_equal))); + // Infinity in tested range + ASSERT_FALSE(a->RangeEquals(b, 0, 2, 0)); + ASSERT_FALSE(b->RangeEquals(a, 0, 2, 0)); + ASSERT_FALSE(a->RangeEquals(b, 1, 2, 1)); + ASSERT_FALSE(b->RangeEquals(a, 1, 2, 1)); + // Infinity not in tested range + ASSERT_TRUE(a->RangeEquals(b, 0, 1, 0)); + ASSERT_TRUE(b->RangeEquals(a, 0, 1, 0)); + } +} + TEST(TestPrimitiveAdHoc, FloatingApproxEquals) { CheckApproxEquals(); CheckApproxEquals(); @@ -1535,6 +1602,11 @@ TEST(TestPrimitiveAdHoc, FloatingNanEquality) { CheckFloatingNanEquality(); } +TEST(TestPrimitiveAdHoc, FloatingInfinityEquality) { + CheckFloatingInfinityEquality(); + CheckFloatingInfinityEquality(); +} + // ---------------------------------------------------------------------- // FixedSizeBinary tests diff --git a/cpp/src/arrow/compare.cc b/cpp/src/arrow/compare.cc index a4cea6f4da4..e0c23a31eac 100644 --- a/cpp/src/arrow/compare.cc +++ b/cpp/src/arrow/compare.cc @@ -110,11 +110,12 @@ inline bool FloatingApproxEquals(const NumericArray& left, if (opts.nans_equal()) { return BaseFloatingEquals(left, right, [epsilon](T x, T y) -> bool { - return (fabs(x - y) <= epsilon) || (std::isnan(x) && std::isnan(y)); + return (fabs(x - y) <= epsilon) || (x == y) || (std::isnan(x) && std::isnan(y)); }); } else { - return BaseFloatingEquals( - left, right, [epsilon](T x, T y) -> bool { return fabs(x - y) <= epsilon; }); + return BaseFloatingEquals(left, right, [epsilon](T x, T y) -> bool { + return (fabs(x - y) <= epsilon) || (x == y); + }); } }