Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions velox/docs/functions/spark/math.rst
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,11 @@ Mathematical Functions

Returns the negative of `x`. Corresponds to Spark's operator ``-``.

For integral types, when ``spark.ansi.enabled`` is true, throws an
arithmetic error if `x` is the minimum value of its type (e.g., -128 for
tinyint). When ``spark.ansi.enabled`` is false, returns the minimum value
unchanged.

.. spark:function:: unhex(x) -> varbinary

Converts hexadecimal varchar ``x`` to varbinary.
Expand Down
31 changes: 25 additions & 6 deletions velox/functions/sparksql/Arithmetic.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
template <typename T>
FOLLY_ALWAYS_INLINE void initialize(
const std::vector<TypePtr>& /*inputTypes*/,
const core::QueryConfig& config,

Check failure on line 37 in velox/functions/sparksql/Arithmetic.h

View workflow job for this annotation

GitHub Actions / Build with GCC / Linux release with adapters

clang-diagnostic-error

use of undeclared identifier 'core'
const T* /*a*/) {
ansiEnabled_ = config.sparkAnsiEnabled();
}
Expand Down Expand Up @@ -146,15 +146,34 @@
template <typename T>
struct UnaryMinusFunction {
template <typename TInput>
FOLLY_ALWAYS_INLINE bool call(TInput& result, const TInput a) {
FOLLY_ALWAYS_INLINE void initialize(
const std::vector<TypePtr>& /*inputTypes*/,
const core::QueryConfig& config,

Check failure on line 151 in velox/functions/sparksql/Arithmetic.h

View workflow job for this annotation

GitHub Actions / Build with GCC / Linux release with adapters

clang-diagnostic-error

use of undeclared identifier 'core'
const TInput* /*a*/) {
ansiEnabled_ = config.sparkAnsiEnabled();
}

template <typename TInput>
FOLLY_ALWAYS_INLINE Status call(TInput& result, const TInput a) {
if constexpr (std::is_integral_v<TInput>) {
// Avoid undefined integer overflow.
result = a == std::numeric_limits<TInput>::min() ? a : -a;
} else {
result = -a;
if (FOLLY_UNLIKELY(a == std::numeric_limits<TInput>::min())) {
if (ansiEnabled_) {
if (threadSkipErrorDetails()) {
return Status::UserError();
}
return Status::UserError("Arithmetic overflow: -({}).", a);
}
// In non-ANSI mode, returns the same negative minimum value.
result = a;
return Status::OK();
}
}
return true;
result = -a;
return Status::OK();
}

private:
bool ansiEnabled_ = false;
};

template <typename T>
Expand Down
33 changes: 29 additions & 4 deletions velox/functions/sparksql/tests/ArithmeticTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -310,14 +310,39 @@
}

TEST_F(ArithmeticTest, UnaryMinusOverflow) {
// Float/double cases are unaffected by ANSI mode.
for (const auto& ansiEnabled : {"false", "true"}) {
queryCtx_->testingOverrideConfigUnsafe(
{{core::QueryConfig::kSparkAnsiEnabled, ansiEnabled}});

Check warning on line 316 in velox/functions/sparksql/tests/ArithmeticTest.cpp

View workflow job for this annotation

GitHub Actions / Build with GCC / Linux release with adapters

misc-include-cleaner

no header providing "facebook::velox::core::QueryConfig" is directly included

EXPECT_EQ(unaryminus<float>(-kInf), kInf);
EXPECT_TRUE(std::isnan(unaryminus<float>(kNan).value_or(0)));
EXPECT_EQ(unaryminus<double>(-kInf), kInf);
EXPECT_TRUE(std::isnan(unaryminus<double>(kNan).value_or(0)));
}

// With ANSI off, negating MIN returns MIN (wraps silently).
queryCtx_->testingOverrideConfigUnsafe(
{{core::QueryConfig::kSparkAnsiEnabled, "false"}});

EXPECT_EQ(unaryminus<int8_t>(INT8_MIN), INT8_MIN);
EXPECT_EQ(unaryminus<int16_t>(INT16_MIN), INT16_MIN);
EXPECT_EQ(unaryminus<int32_t>(INT32_MIN), INT32_MIN);
EXPECT_EQ(unaryminus<int64_t>(INT64_MIN), INT64_MIN);
EXPECT_EQ(unaryminus<float>(-kInf), kInf);
EXPECT_TRUE(std::isnan(unaryminus<float>(kNan).value_or(0)));
EXPECT_EQ(unaryminus<double>(-kInf), kInf);
EXPECT_TRUE(std::isnan(unaryminus<double>(kNan).value_or(0)));

// With ANSI on, negating MIN throws.
queryCtx_->testingOverrideConfigUnsafe(
{{core::QueryConfig::kSparkAnsiEnabled, "true"}});

VELOX_ASSERT_THROW(unaryminus<int8_t>(INT8_MIN), "Arithmetic overflow");
VELOX_ASSERT_THROW(unaryminus<int16_t>(INT16_MIN), "Arithmetic overflow");
VELOX_ASSERT_THROW(unaryminus<int32_t>(INT32_MIN), "Arithmetic overflow");
VELOX_ASSERT_THROW(unaryminus<int64_t>(INT64_MIN), "Arithmetic overflow");

// TRY wrapping returns null instead of throwing.
auto tryResult =
evaluateOnce<int64_t>("try(unaryminus(c0))", std::optional(INT64_MIN));
EXPECT_FALSE(tryResult.has_value());

Check warning on line 345 in velox/functions/sparksql/tests/ArithmeticTest.cpp

View workflow job for this annotation

GitHub Actions / Build with GCC / Linux release with adapters

misc-include-cleaner

no header providing "EXPECT_FALSE" is directly included
}

TEST_F(ArithmeticTest, Divide) {
Expand Down
Loading