Skip to content

Commit

Permalink
Throw user error for decimal overflow in decimal divide Presto functi…
Browse files Browse the repository at this point in the history
…on (facebookincubator#9632)

Summary:
Decimal divide fails in fuzzer when the result scale exceeds 38. Changes to
throw user error instead.

Pull Request resolved: facebookincubator#9632

Reviewed By: Yuhta

Differential Revision: D56640827

Pulled By: kagamiori

fbshipit-source-id: 955c468dc1eb04b493f7e64737dc1ff59ab3e000
  • Loading branch information
rui-mo authored and facebook-github-bot committed Apr 26, 2024
1 parent f9c8fd4 commit c2526ba
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 12 deletions.
2 changes: 1 addition & 1 deletion velox/functions/prestosql/DecimalFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ struct DecimalDivideFunction {
auto bScale = getDecimalPrecisionScale(*bType).second;
auto rScale = std::max(aScale, bScale);
aRescale_ = rScale - aScale + bScale;
VELOX_CHECK_LE(
VELOX_USER_CHECK_LE(
aRescale_, LongDecimalType::kMaxPrecision, "Decimal overflow");
}

Expand Down
22 changes: 11 additions & 11 deletions velox/functions/prestosql/tests/DecimalArithmeticTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ TEST_F(DecimalArithmeticTest, add) {
{1, 2, 5, std::nullopt, std::nullopt}, DECIMAL(10, 3))});

// Addition overflow.
VELOX_ASSERT_THROW(
VELOX_ASSERT_USER_THROW(
testDecimalExpr<TypeKind::HUGEINT>(
{},
"c0 + cast(1.00 as decimal(2,0))",
Expand All @@ -126,7 +126,7 @@ TEST_F(DecimalArithmeticTest, add) {
"Decimal overflow. Value '100000000000000000000000000000000000000' is not in the range of Decimal Type");

// Rescaling LHS overflows.
VELOX_ASSERT_THROW(
VELOX_ASSERT_USER_THROW(
testDecimalExpr<TypeKind::HUGEINT>(
{},
"c0 + 0.01",
Expand All @@ -135,7 +135,7 @@ TEST_F(DecimalArithmeticTest, add) {
DECIMAL(38, 0))}),
"Decimal overflow: 99999999999999999999999999999999999999 + 1");
// Rescaling RHS overflows.
VELOX_ASSERT_THROW(
VELOX_ASSERT_USER_THROW(
testDecimalExpr<TypeKind::HUGEINT>(
{},
"0.01 + c0",
Expand Down Expand Up @@ -202,7 +202,7 @@ TEST_F(DecimalArithmeticTest, subtract) {
{1, 2, 5, std::nullopt, std::nullopt}, DECIMAL(10, 3))});

// Subtraction overflow.
VELOX_ASSERT_THROW(
VELOX_ASSERT_USER_THROW(
testDecimalExpr<TypeKind::HUGEINT>(
{},
"c0 - cast(1.00 as decimal(2,0))",
Expand All @@ -211,7 +211,7 @@ TEST_F(DecimalArithmeticTest, subtract) {
DECIMAL(38, 0))}),
"Decimal overflow. Value '-100000000000000000000000000000000000000' is not in the range of Decimal Type");
// Rescaling LHS overflows.
VELOX_ASSERT_THROW(
VELOX_ASSERT_USER_THROW(
testDecimalExpr<TypeKind::HUGEINT>(
{},
"c0 - 0.01",
Expand All @@ -220,7 +220,7 @@ TEST_F(DecimalArithmeticTest, subtract) {
DECIMAL(38, 0))}),
"Decimal overflow: -99999999999999999999999999999999999999 - 1");
// Rescaling RHS overflows.
VELOX_ASSERT_THROW(
VELOX_ASSERT_USER_THROW(
testDecimalExpr<TypeKind::HUGEINT>(
{},
"0.01 - c0",
Expand Down Expand Up @@ -271,7 +271,7 @@ TEST_F(DecimalArithmeticTest, multiply) {
expectedConstantFlat, "c0 * 1.00", {shortFlat});

// Long decimal limits
VELOX_ASSERT_THROW(
VELOX_ASSERT_USER_THROW(
testDecimalExpr<TypeKind::HUGEINT>(
{},
"c0 * cast(10.00 as decimal(2,0))",
Expand All @@ -282,7 +282,7 @@ TEST_F(DecimalArithmeticTest, multiply) {
"Decimal overflow. Value '119630519620642428561342635425231011830' is not in the range of Decimal Type");

// Rescaling the final result overflows.
VELOX_ASSERT_THROW(
VELOX_ASSERT_USER_THROW(
testDecimalExpr<TypeKind::HUGEINT>(
{},
"c0 * cast(1.00 as decimal(2,1))",
Expand Down Expand Up @@ -365,12 +365,12 @@ TEST_F(DecimalArithmeticTest, decimalDivTest) {
{makeFlatVector<int64_t>({-34, 5, 65, 90, 2, -49}, DECIMAL(2, 1))});

// Divide by zero.
VELOX_ASSERT_THROW(
VELOX_ASSERT_USER_THROW(
testDecimalExpr<TypeKind::BIGINT>({}, "c0 / 0.0", {shortFlat}),
"Division by zero");

// Long decimal limits.
VELOX_ASSERT_THROW(
VELOX_ASSERT_USER_THROW(
testDecimalExpr<TypeKind::HUGEINT>(
{},
"c0 / 0.01",
Expand All @@ -380,7 +380,7 @@ TEST_F(DecimalArithmeticTest, decimalDivTest) {
"Decimal overflow: 99999999999999999999999999999999999999 * 10000");

// Rescale factor > max precision (38).
VELOX_ASSERT_THROW(
VELOX_ASSERT_USER_THROW(
evaluate(
"divide(c0, c1)",
makeRowVector(
Expand Down

0 comments on commit c2526ba

Please sign in to comment.