yaooqinn commented on a change in pull request #26995: [SPARK-30341][SQL]
Overflow check for interval arithmetic operations
URL: https://github.com/apache/spark/pull/26995#discussion_r361675825
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala
##########
@@ -413,6 +415,49 @@ object IntervalUtils {
new CalendarInterval(truncatedMonths, truncatedDays, micros.round)
}
+ /**
+ * Makes an interval from months, days and micros with the fractional part by
+ * adding the month fraction to days and the days fraction to micros.
+ */
+ private def safeFromDoubles(
+ monthsWithFraction: Double,
+ daysWithFraction: Double,
+ microsWithFraction: Double): CalendarInterval = {
+ val monthInLong = monthsWithFraction.toLong
+ val truncatedMonths = if (monthInLong > Int.MaxValue) {
Review comment:
`WHEN ANSI OFF`
1. If we use `NULL ON OVERFLOW` policy for all arithmetics, means interval
`add/subtract/negate` break their backward compatibility to fit
`multiply/divide`, seem not a good choice.
2. If we remain to use `NULL ON OVERFLOW` policy for `multiply/divide` and
java overflow style for `add `etc, this breaks no backward compatibility but
also keeps the inconsistency. seem not a good choice either.
3. If we throw exceptions for `multiply/divide` for overflows and remain
java overflow style for `add` etc, this breaks no backward compatibility but
also raises much worse inconsistency issue.
4. If we throw exceptions for all interval arithmetics, this is just `ANSI
ON`.
5. Behavior of current implementation, we do not break backward
compatibility either. Things only go "wrong" only when it reaches the
`CalendarInterval.MAX_VALUE` or `CalendarInterval.MIN_VALUE`. It seems the best
choice among the worst.
Things became odd since we didn't support fraction representations for
intervals, but support `multiply/divide` intervals with fractions. a.k.a we do
not have consistent rules https://github.com/apache/spark/pull/26592 for how
the fraction part of month go days, how day fraction go milliseconds.
There seems no other systems can refer to for our `ANSI OFF` mode.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
