From dbcbc42628766fe1672135b338cfcf6b6423b188 Mon Sep 17 00:00:00 2001 From: Bikramjeet Vig Date: Wed, 24 Jul 2024 16:07:32 -0700 Subject: [PATCH] Fix isFinite for NaN input Summary: isFinite() currently incorrectly classifies NaN as finite. This fixes that bug. Reviewed By: amitkdutta Differential Revision: D60204120 --- velox/functions/prestosql/Arithmetic.h | 2 +- velox/functions/prestosql/tests/ArithmeticTest.cpp | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/velox/functions/prestosql/Arithmetic.h b/velox/functions/prestosql/Arithmetic.h index 3757284bf2f4..7d3c44d894c4 100644 --- a/velox/functions/prestosql/Arithmetic.h +++ b/velox/functions/prestosql/Arithmetic.h @@ -451,7 +451,7 @@ struct InfinityFunction { template struct IsFiniteFunction { FOLLY_ALWAYS_INLINE void call(bool& result, double a) { - result = !std::isinf(a); + result = std::isfinite(a); } }; diff --git a/velox/functions/prestosql/tests/ArithmeticTest.cpp b/velox/functions/prestosql/tests/ArithmeticTest.cpp index 953f8ef7e3b9..36bffb0c10c9 100644 --- a/velox/functions/prestosql/tests/ArithmeticTest.cpp +++ b/velox/functions/prestosql/tests/ArithmeticTest.cpp @@ -599,6 +599,7 @@ TEST_F(ArithmeticTest, isFinite) { EXPECT_EQ(false, isFinite(-kInf)); EXPECT_EQ(false, isFinite(1.0 / 0.0)); EXPECT_EQ(false, isFinite(-1.0 / 0.0)); + EXPECT_EQ(false, isFinite(kNan)); } TEST_F(ArithmeticTest, isInfinite) { @@ -607,6 +608,7 @@ TEST_F(ArithmeticTest, isInfinite) { }; EXPECT_EQ(false, isInfinite(0.0)); + EXPECT_EQ(false, isInfinite(kNan)); EXPECT_EQ(true, isInfinite(kInf)); EXPECT_EQ(true, isInfinite(-kInf)); EXPECT_EQ(true, isInfinite(1.0 / 0.0));