MaxGekk commented on code in PR #59956:
URL: https://github.com/apache/spark/pull/59956#discussion_r4426263374
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/intervalExpressions.scala:
##########
@@ -788,11 +836,15 @@ case class DivideYMInterval(
LongMath.divide(months, num.asInstanceOf[Long],
RoundingMode.HALF_UP).toInt
case _: IntegralType => (months: Int, num) =>
IntMath.divide(months, num.asInstanceOf[Number].intValue(),
RoundingMode.HALF_UP)
- case _: DecimalType => (months: Int, num) =>
+ case _: DecimalType => (months: Int, num) =>
checkRoundingOverflow(origin.context) {
val decimalRes = ((new Decimal).set(months) /
num.asInstanceOf[Decimal]).toJavaBigDecimal
decimalRes.setScale(0, java.math.RoundingMode.HALF_UP).intValueExact()
+ }
case _: FractionalType => (months: Int, num) =>
- DoubleMath.roundToInt(months / num.asInstanceOf[Number].doubleValue(),
RoundingMode.HALF_UP)
+ val quotient = months / num.asInstanceOf[Number].doubleValue()
+ checkRoundingOverflow(origin.context,
java.lang.Double.isFinite(quotient)) {
Review Comment:
Good catch, that was my oversight when scoping the catch to keep the NaN
tests passing. In bda6facf7bc only a NaN quotient, which only a NaN divisor
gives, is left to the rounding, and an infinite quotient raises the interval
overflow error, so `INTERVAL '1' DAY / 1e-300D` reports
`INTERVAL_ARITHMETIC_OVERFLOW.WITH_SUGGESTION` like `/ 1e-290D`. The check now
lives in `IntervalMathUtils.divide` (item 5), so there is one copy of it per
interval type.
##########
docs/sql-migration-guide.md:
##########
@@ -28,6 +28,7 @@ license: |
## Upgrading from Spark SQL 4.3 to 4.4
+- Since Spark 4.4, dividing an ANSI interval by a number raises
`INTERVAL_ARITHMETIC_OVERFLOW.WITH_SUGGESTION` (SQLSTATE 22015) whenever the
result does not fit the interval type, in every evaluation mode. Previously,
dividing the minimum year-month or day-time interval by `-1` raised
`ARITHMETIC_OVERFLOW` (SQLSTATE 22003) under whole-stage codegen, and dividing
an interval by a decimal or a floating-point number whose rounded quotient
overflows the type raised a `java.lang.ArithmeticException` without an error
condition.
Review Comment:
Fixed in bda6facf7bc: with item 1 the sentence holds, and the note now says
"in generated code" instead of "under whole-stage codegen". The PR description
also notes that a projection uses generated code with the default factory mode
even with whole-stage codegen off.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/IntervalExpressionsSuite.scala:
##########
@@ -476,6 +476,26 @@ class IntervalExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
}
}
+ test("SPARK-60746: the min interval divided by -1 raises the same error in
every mode") {
+ // An integral divisor is checked before the division, a decimal or a
fractional one when the
+ // rounded quotient does not fit the interval type. The error carries an
empty query context,
+ // which `checkErrorInExpression` cannot match, so the whole message is
checked instead,
+ // including the suggested function.
+ val expectedErrMsg = "[INTERVAL_ARITHMETIC_OVERFLOW.WITH_SUGGESTION]
Integer overflow " +
+ "while operating with intervals. Use `try_divide` to tolerate overflow
and return NULL " +
+ "instead."
+ Seq(-1.toByte, -1.toShort, -1, -1L, Decimal(-1), -1.0f, -1.0d).foreach {
num =>
Review Comment:
Added in bda6facf7bc, in a separate test as you suggested:
`Period.ofMonths(Int.MaxValue) / 1e-300D`, `Duration.ofDays(1) / 1e-300D`
(infinite quotients), `Duration.ofDays(1) / 1e-290D` (a day-time overflow with
a fractional divisor) and `Period.ofMonths(Int.MaxValue) / 0.5` (a decimal
one), in both modes. The NaN cases of the SPARK-34868 and SPARK-34875 tests
keep their rounding error.
--
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]
