viirya commented on code in PR #59334:
URL: https://github.com/apache/spark/pull/59334#discussion_r4238636780
##########
sql/api/src/main/scala/org/apache/spark/sql/types/Decimal.scala:
##########
@@ -635,8 +635,9 @@ object Decimal {
}
}
- private def numDigitsInIntegralPart(bigDecimal: JavaBigDecimal): Int =
- bigDecimal.precision - bigDecimal.scale
+ // Computed in Long since `precision - scale` can overflow Int, e.g. for
"1e2147483647".
+ private def numDigitsInIntegralPart(bigDecimal: JavaBigDecimal): Long =
+ bigDecimal.precision.toLong - bigDecimal.scale
Review Comment:
This fails in `changePrecision` rather than in the fast-fail, so it's
handled in #59335 (SPARK-60119), which bounds the small side there.
##########
sql/api/src/main/scala/org/apache/spark/sql/types/Decimal.scala:
##########
@@ -635,8 +635,9 @@ object Decimal {
}
}
- private def numDigitsInIntegralPart(bigDecimal: JavaBigDecimal): Int =
- bigDecimal.precision - bigDecimal.scale
+ // Computed in Long since `precision - scale` can overflow Int, e.g. for
"1e2147483647".
+ private def numDigitsInIntegralPart(bigDecimal: JavaBigDecimal): Long =
Review Comment:
Covered in #59335 (SPARK-60119), with a cast-level test under the legacy
conf. I noted it in the PR description.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastWithAnsiOnSuite.scala:
##########
@@ -475,6 +475,11 @@ class CastWithAnsiOnSuite extends CastSuiteBase with
QueryErrorsBase {
checkExceptionInExpression[ArithmeticException](
cast("6E+37", DecimalType(38, 1)),
"cannot be represented as Decimal(38, 1)")
+ Seq("1e2147483647", "0e2147483647", "1.5e2147483647",
"1e2147483648").foreach { s =>
Review Comment:
Good point. Done: the fast-fail skips zero now, and the tests check that
`'0e39'`, `'0e2147483647'` and `'0.0e2147483648'` cast to `0.00`.
##########
sql/api/src/main/scala/org/apache/spark/sql/types/Decimal.scala:
##########
@@ -635,8 +635,9 @@ object Decimal {
}
}
- private def numDigitsInIntegralPart(bigDecimal: JavaBigDecimal): Int =
- bigDecimal.precision - bigDecimal.scale
+ // Computed in Long since `precision - scale` can overflow Int, e.g. for
"1e2147483647".
Review Comment:
Same fix as above in #59335; `'1e-100000000'` returns `0.00` without the
`setScale` cost.
##########
sql/api/src/main/scala/org/apache/spark/sql/types/Decimal.scala:
##########
@@ -635,8 +635,9 @@ object Decimal {
}
}
- private def numDigitsInIntegralPart(bigDecimal: JavaBigDecimal): Int =
- bigDecimal.precision - bigDecimal.scale
+ // Computed in Long since `precision - scale` can overflow Int, e.g. for
"1e2147483647".
+ private def numDigitsInIntegralPart(bigDecimal: JavaBigDecimal): Long =
Review Comment:
Filed SPARK-60132 for this.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/types/DecimalSuite.scala:
##########
@@ -446,6 +446,19 @@ class DecimalSuite extends SparkFunSuite with
PrivateMethodTester with SQLHelper
}
}
+ test("UTF8String to Decimal with exponent near Int.MaxValue") {
Review Comment:
The negative-exponent and legacy-conf cases are tested in #59335. I added
zero cases here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
