diff --git a/velox/functions/sparksql/tests/SparkCastExprTest.cpp b/velox/functions/sparksql/tests/SparkCastExprTest.cpp index c185b3cba76b0..1f910a1ea96bf 100644 --- a/velox/functions/sparksql/tests/SparkCastExprTest.cpp +++ b/velox/functions/sparksql/tests/SparkCastExprTest.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include "velox/common/base/tests/GTestUtils.h" #include "velox/functions/prestosql/tests/CastBaseTest.h" #include "velox/functions/sparksql/Register.h" #include "velox/parse/TypeResolver.h" @@ -453,6 +454,17 @@ TEST_F(SparkCastExprTest, overflow) { testCast( makeNullableFlatVector({214748364890}, DECIMAL(12, 2)), makeNullableFlatVector({2147483648})); + + VELOX_ASSERT_THROW( + (testCast("tinyint", {"128"}, {std::nullopt})), + "Cannot cast VARCHAR '128' to INTEGER. Value is too large for type."); + testTryCast("tinyint", {"128"}, {std::nullopt}); + + VELOX_ASSERT_THROW( + (testCast( + "integer", {"17515055537"}, {std::nullopt})), + "Cannot cast VARCHAR '17515055537' to INTEGER. Value is too large for type."); + testTryCast("integer", {"17515055537"}, {std::nullopt}); } TEST_F(SparkCastExprTest, timestampToString) { diff --git a/velox/type/Conversions.h b/velox/type/Conversions.h index 0377d14be338c..2b883e0e7ec54 100644 --- a/velox/type/Conversions.h +++ b/velox/type/Conversions.h @@ -195,11 +195,14 @@ struct Converter< VELOX_USER_FAIL("Encountered a non-digit character"); } if (!decimalPoint) { - result = result * 10 - (v[index] - '0'); - } - // Overflow check - if (result > 0) { - VELOX_USER_FAIL("Value is too large for type"); + bool overflow = __builtin_mul_overflow(result, 10, &result); + if (overflow) { + VELOX_USER_FAIL("Value is too large for type."); + } + overflow = __builtin_sub_overflow(result, v[index] - '0', &result); + if (overflow) { + VELOX_USER_FAIL("Value is too large for type."); + } } } } else { @@ -215,11 +218,14 @@ struct Converter< VELOX_USER_FAIL("Encountered a non-digit character"); } if (!decimalPoint) { - result = result * 10 + (v[index] - '0'); - } - // Overflow check - if (result < 0) { - VELOX_USER_FAIL("Value is too large for type"); + bool overflow = __builtin_mul_overflow(result, 10, &result); + if (overflow) { + VELOX_USER_FAIL("Value is too large for type."); + } + overflow = __builtin_add_overflow(result, v[index] - '0', &result); + if (overflow) { + VELOX_USER_FAIL("Value is too large for type."); + } } } }