From bb5459fb26b9d0d57eadee8b10b7488607eeaeb2 Mon Sep 17 00:00:00 2001 From: Angerszhuuuu Date: Thu, 22 Apr 2021 11:59:38 +0300 Subject: [PATCH] [SPARK-35177][SQL] Fix arithmetic overflow in parsing the minimal interval by `IntervalUtils.fromYearMonthString` ### What changes were proposed in this pull request? IntervalUtils.fromYearMonthString should handle Int.MinValue months correctly. In current logic, just use `Math.addExact(Math.multiplyExact(years, 12), months)` to calculate negative total months will overflow when actual total months is Int.MinValue, this pr fixes this bug. ### Why are the changes needed? IntervalUtils.fromYearMonthString should handle Int.MinValue months correctly ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Added UT Closes #32281 from AngersZhuuuu/SPARK-35177. Authored-by: Angerszhuuuu Signed-off-by: Max Gekk --- .../spark/sql/catalyst/util/IntervalUtils.scala | 13 ++++++------- .../sql/catalyst/util/IntervalUtilsSuite.scala | 13 +++++++++++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala index e52d3c8817..03b1835c7c 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala @@ -100,12 +100,11 @@ object IntervalUtils { */ def fromYearMonthString(input: String): CalendarInterval = { require(input != null, "Interval year-month string must be not null") - def toInterval(yearStr: String, monthStr: String): CalendarInterval = { + def toInterval(yearStr: String, monthStr: String, sign: Int): CalendarInterval = { try { - val years = toLongWithRange(YEAR, yearStr, 0, Integer.MAX_VALUE).toInt - val months = toLongWithRange(MONTH, monthStr, 0, 11).toInt - val totalMonths = Math.addExact(Math.multiplyExact(years, 12), months) - new CalendarInterval(totalMonths, 0, 0) + val years = toLongWithRange(YEAR, yearStr, 0, Integer.MAX_VALUE / MONTHS_PER_YEAR) + val totalMonths = sign * (years * MONTHS_PER_YEAR + toLongWithRange(MONTH, monthStr, 0, 11)) + new CalendarInterval(Math.toIntExact(totalMonths), 0, 0) } catch { case NonFatal(e) => throw new IllegalArgumentException( @@ -114,9 +113,9 @@ object IntervalUtils { } input.trim match { case yearMonthPattern("-", yearStr, monthStr) => - negateExact(toInterval(yearStr, monthStr)) + toInterval(yearStr, monthStr, -1) case yearMonthPattern(_, yearStr, monthStr) => - toInterval(yearStr, monthStr) + toInterval(yearStr, monthStr, 1) case _ => throw new IllegalArgumentException( s"Interval string does not match year-month format of 'y-m': $input") diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala index 5c460f70a9..87d306a495 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala @@ -169,6 +169,19 @@ class IntervalUtilsSuite extends SparkFunSuite with SQLHelper { fromYearMonthString) failFuncWithInvalidInput("-\t99-15", "Interval string does not match year-month format", fromYearMonthString) + + assert(fromYearMonthString("178956970-6") == new CalendarInterval(Int.MaxValue - 1, 0, 0)) + assert(fromYearMonthString("178956970-7") == new CalendarInterval(Int.MaxValue, 0, 0)) + + val e1 = intercept[IllegalArgumentException]{ + assert(fromYearMonthString("178956970-8") == new CalendarInterval(Int.MinValue, 0, 0)) + }.getMessage + assert(e1.contains("integer overflow")) + assert(fromYearMonthString("-178956970-8") == new CalendarInterval(Int.MinValue, 0, 0)) + val e2 = intercept[IllegalArgumentException]{ + assert(fromYearMonthString("-178956970-9") == new CalendarInterval(Int.MinValue, 0, 0)) + }.getMessage + assert(e2.contains("integer overflow")) } test("from day-time string - legacy") {