Skip to content

Commit

Permalink
Fix power function NaN handling so it returns NaN (#11210)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #11210

The C++ std::pow function returns 1 when 1 is raised to NaN. It returns NaN for every other number except for 1 which is strange. This change makes sure it will return NaN.

Query: select pow(c0, nan()) from (values 1.0) t(c0);
Presto-0.289: Returns NaN
Velox: Returns 1.0

Reviewed By: kgpai

Differential Revision: D64145275

fbshipit-source-id: 2d4890ad47b2c140867328778d7102733cd1cb32
  • Loading branch information
Daniel Hunte authored and facebook-github-bot committed Oct 10, 2024
1 parent a6899b7 commit a976ba5
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 3 deletions.
4 changes: 3 additions & 1 deletion velox/functions/prestosql/Arithmetic.h
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,9 @@ struct PowerFunction {
template <typename TInput>
FOLLY_ALWAYS_INLINE void
call(double& result, const TInput& a, const TInput& b) {
result = std::pow(a, b);
result = (std::isnan(a) && b != 0) || std::isnan(b) || std::isinf(b)
? std::numeric_limits<double>::quiet_NaN()
: pow(a, b);
}
};

Expand Down
28 changes: 26 additions & 2 deletions velox/functions/prestosql/tests/ArithmeticTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,9 @@ TEST_F(ArithmeticTest, modInt) {

TEST_F(ArithmeticTest, power) {
std::vector<double> baseDouble = {
0, 0, 0, -1, -1, -1, -9, 9.1, 10.1, 11.1, -11.1, 0, kInf, kInf};
0, 0, 0, -1, -1, -1, -9, 9.1, 10.1, 11.1, -11.1};
std::vector<double> exponentDouble = {
0, 1, -1, 0, 1, -1, -3.3, 123456.432, -99.9, 0, 100000, kInf, 0, kInf};
0, 1, -1, 0, 1, -1, -3.3, 123456.432, -99.9, 0, 100000};
std::vector<double> expectedDouble;
expectedDouble.reserve(baseDouble.size());

Expand All @@ -279,6 +279,30 @@ TEST_F(ArithmeticTest, power) {
"pow(c0, c1)", baseDouble, exponentDouble, expectedDouble);
}

TEST_F(ArithmeticTest, powerNan) {
std::vector<double> baseDouble = {1, kNan, kNan, kNan};
std::vector<double> exponentDouble = {kNan, 1, kInf, 0};
std::vector<double> expectedDouble = {kNan, kNan, kNan, 1};

// Check using function name and alias.
assertExpression<double>(
"power(c0, c1)", baseDouble, exponentDouble, expectedDouble);
assertExpression<double>(
"pow(c0, c1)", baseDouble, exponentDouble, expectedDouble);
}

TEST_F(ArithmeticTest, powerInf) {
std::vector<double> baseDouble = {1, kInf, kInf, kInf};
std::vector<double> exponentDouble = {kInf, 1, kNan, 0};
std::vector<double> expectedDouble = {kNan, kInf, kNan, 1};

// Check using function name and alias.
assertExpression<double>(
"power(c0, c1)", baseDouble, exponentDouble, expectedDouble);
assertExpression<double>(
"pow(c0, c1)", baseDouble, exponentDouble, expectedDouble);
}

TEST_F(ArithmeticTest, powerInt) {
std::vector<int64_t> baseInt = {9, 10, 11, -9, -10, -11, 0};
std::vector<int64_t> exponentInt = {3, -3, 0, -1, 199999, 77, 0};
Expand Down

0 comments on commit a976ba5

Please sign in to comment.