[SPARK-29864][SPARK-29920][SQL] Strict parsing of day-time strings to intervals

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala

@@ -23,6 +23,7 @@ import scala.util.control.NonFatal
 
 import org.apache.spark.sql.catalyst.parser.{CatalystSqlParser, ParseException}
 import org.apache.spark.sql.catalyst.util.DateTimeConstants._
+import org.apache.spark.sql.internal.SQLConf
 import org.apache.spark.sql.types.Decimal
 import org.apache.spark.unsafe.types.{CalendarInterval, UTF8String}
 
@@ -183,9 +184,6 @@ object IntervalUtils {
     fromDayTimeString(s, DAY, SECOND)
   }
 
-  private val dayTimePattern =
-    "^([+|-])?((\\d+) )?((\\d+):)?(\\d+):(\\d+)(\\.(\\d+))?$".r
-
   /**
    * Parse dayTime string in form: [-]d HH:mm:ss.nnnnnnnnn and [-]HH:mm:ss.nnnnnnnnn
    *
@@ -196,9 +194,23 @@ object IntervalUtils {
    * - MINUTE TO SECOND
    */
   def fromDayTimeString(input: String, from: IntervalUnit, to: IntervalUnit): CalendarInterval = {
+    if (SQLConf.get.getConf(SQLConf.LEGACY_FROM_DAYTIME_STRING)) {
+      fromDayTimeLegacy(input, from, to)
+    } else {
+      fromDayTime(input, from, to)
+    }
+  }
+
+  private val dayTimePatternLegacy =
+    "^([+|-])?((\\d+) )?((\\d+):)?(\\d+):(\\d+)(\\.(\\d+))?$".r
+
+  private def fromDayTimeLegacy(
+      input: String,
+      from: IntervalUnit,
+      to: IntervalUnit): CalendarInterval = {
     require(input != null, "Interval day-time string must be not null")
     assert(input.length == input.trim.length)
-    val m = dayTimePattern.pattern.matcher(input)
+    val m = dayTimePatternLegacy.pattern.matcher(input)
     require(m.matches, s"Interval string must match day-time format of 'd h:m:s.n': $input")
 
     try {
@@ -233,7 +245,7 @@ object IntervalUtils {
           seconds = 0
           secondsFraction = 0
         case SECOND =>
-          // No-op
+        // No-op
         case _ =>
           throw new IllegalArgumentException(
             s"Cannot support (interval '$input' $from to $to) expression")
@@ -250,6 +262,66 @@ object IntervalUtils {
     }
   }
 
+  private val signRe = "(?<sign>[+|-])"
+  private val dayRe = "(?<day>\\d+)"
+  private val hourRe = "(?<hour>\\d{1,2})"
+  private val minuteRe = "(?<minute>\\d{1,2})"
+  private val secondRe = "(?<second>(\\d{1,2})(\\.(\\d{1,9}))?)"
+
+  private val dayTimePattern = Map(
+    (MINUTE, SECOND) -> s"^$signRe?$minuteRe:$secondRe$$".r,
+    (HOUR, MINUTE) -> s"^$signRe?$hourRe:$minuteRe$$".r,
+    (HOUR, SECOND) -> s"^$signRe?$hourRe:$minuteRe:$secondRe$$".r,
+    (DAY, HOUR) -> s"^$signRe?$dayRe $hourRe$$".r,
+    (DAY, MINUTE) -> s"^$signRe?$dayRe $hourRe:$minuteRe$$".r,
+    (DAY, SECOND) -> s"^$signRe?$dayRe $hourRe:$minuteRe:$secondRe$$".r
+  )
+
+  private def unitsRange(start: IntervalUnit, end: IntervalUnit): Seq[IntervalUnit] = {
+    (start.id to end.id).map(IntervalUnit(_))
+  }
+
+  private def fromDayTime(
+      input: String,
+      from: IntervalUnit,
+      to: IntervalUnit): CalendarInterval = {
+    require(input != null, "Interval day-time string must be not null")
+    val regexp = dayTimePattern.get(from -> to)
+    require(regexp.isDefined, s"Cannot support (interval '$input' $from to $to) expression")
+    val pattern = regexp.get.pattern
+    val m = pattern.matcher(input)
+    require(m.matches, s"Interval string must match day-time format of '$pattern': $input")
+
+    try {
+      var micros: Long = 0L
+      var days: Int = 0
+
+      unitsRange(to, from).foreach {
+        case unit @ DAY =>
+          val parsed = toLongWithRange(unit, m.group(unit.toString), 0, Int.MaxValue)
+          days = Math.toIntExact(parsed)
+        case unit @ HOUR =>
+          val parsed = toLongWithRange(unit, m.group(unit.toString), 0, 23)
+          micros = Math.addExact(micros, Math.multiplyExact(parsed, MICROS_PER_HOUR))
+        case unit @ MINUTE =>
+          val parsed = toLongWithRange(unit, m.group(unit.toString), 0, 59)
+          micros = Math.addExact(micros, Math.multiplyExact(parsed, MICROS_PER_MINUTE))
+        case unit @ SECOND =>
+          val parsed = parseSecondNano(m.group(unit.toString))
+          micros = Math.addExact(micros, parsed)
+        case _ =>
+          throw new IllegalArgumentException(
+            s"Cannot support (interval '$input' $from to $to) expression")
+      }
+      val sign = if (m.group("sign") != null && m.group("sign") == "-") -1 else 1
+      new CalendarInterval(0, sign * days, sign * micros)
+    } catch {
+      case e: Exception =>
+        throw new IllegalArgumentException(
+          s"Error parsing interval day-time string: ${e.getMessage}", e)
+    }
+  }
+
   def fromUnitStrings(units: Array[IntervalUnit], values: Array[String]): CalendarInterval = {
     assert(units.length == values.length)
     var months: Int = 0

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

@@ -2050,6 +2050,16 @@ object SQLConf {
       .stringConf
       .createWithDefault(
         "https://maven-central.storage-download.googleapis.com/repos/central/data/")
+
+  val LEGACY_FROM_DAYTIME_STRING =
+    buildConf("spark.sql.legacy.fromDayTimeString.enabled")
+      .doc("When true, the `from` bound is not taken into account in conversion of " +
+        "a day-time string to an interval, and the `to` bound is used to skip" +
+        "all interval units out of the specified range. If it is set to `false`, " +
+        "`ParseException` is thrown if the input does not match to the pattern " +
+        "defined by `from` and `to`.")
+      .booleanConf
+      .createWithDefault(false)
 }
 
 /**

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala

@@ -685,7 +685,7 @@ class ExpressionParserSuite extends AnalysisTest {
       "0:0:0",
       "0:0:1")
     hourTimeValues.foreach { value =>
-      val result = Literal(IntervalUtils.fromDayTimeString(value))
+      val result = Literal(IntervalUtils.fromDayTimeString(value, HOUR, SECOND))
       checkIntervals(s"'$value' hour to second", result)
     }
 

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala

@@ -20,12 +20,14 @@ package org.apache.spark.sql.catalyst.util
 import java.util.concurrent.TimeUnit
 
 import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.catalyst.plans.SQLHelper
 import org.apache.spark.sql.catalyst.util.DateTimeConstants._
 import org.apache.spark.sql.catalyst.util.IntervalUtils._
 import org.apache.spark.sql.catalyst.util.IntervalUtils.IntervalUnit._
+import org.apache.spark.sql.internal.SQLConf
 import org.apache.spark.unsafe.types.{CalendarInterval, UTF8String}
 
-class IntervalUtilsSuite extends SparkFunSuite {
+class IntervalUtilsSuite extends SparkFunSuite with SQLHelper {
 
   private def checkFromString(input: String, expected: CalendarInterval): Unit = {
     assert(fromString(input) === expected)
@@ -131,43 +133,45 @@ class IntervalUtilsSuite extends SparkFunSuite {
     }
   }
 
-  test("from day-time string") {
-    assert(fromDayTimeString("5 12:40:30.999999999") ===
-      new CalendarInterval(
-        0,
-        5,
-        12 * MICROS_PER_HOUR +
-        40 * MICROS_PER_MINUTE +
-        30 * MICROS_PER_SECOND + 999999L))
-    assert(fromDayTimeString("10 0:12:0.888") ===
-      new CalendarInterval(
-        0,
-        10,
-        12 * MICROS_PER_MINUTE + 888 * MICROS_PER_MILLIS))
-    assert(fromDayTimeString("-3 0:0:0") === new CalendarInterval(0, -3, 0L))
-
-    try {
-      fromDayTimeString("5 30:12:20")
-      fail("Expected to throw an exception for the invalid input")
-    } catch {
-      case e: IllegalArgumentException =>
-        assert(e.getMessage.contains("hour 30 outside range"))
-    }
-
-    try {
-      fromDayTimeString("5 30-12")
-      fail("Expected to throw an exception for the invalid input")
-    } catch {
-      case e: IllegalArgumentException =>
-        assert(e.getMessage.contains("must match day-time format"))
-    }
-
-    try {
-      fromDayTimeString("5 1:12:20", HOUR, MICROSECOND)
-      fail("Expected to throw an exception for the invalid convention type")
-    } catch {
-      case e: IllegalArgumentException =>
-        assert(e.getMessage.contains("Cannot support (interval"))
+  test("from day-time string - legacy") {
+    withSQLConf(SQLConf.LEGACY_FROM_DAYTIME_STRING.key -> "true") {
+      assert(fromDayTimeString("5 12:40:30.999999999") ===
+        new CalendarInterval(
+          0,
+          5,
+          12 * MICROS_PER_HOUR +
+            40 * MICROS_PER_MINUTE +
+            30 * MICROS_PER_SECOND + 999999L))
+      assert(fromDayTimeString("10 0:12:0.888") ===
+        new CalendarInterval(
+          0,
+          10,
+          12 * MICROS_PER_MINUTE + 888 * MICROS_PER_MILLIS))
+      assert(fromDayTimeString("-3 0:0:0") === new CalendarInterval(0, -3, 0L))
+
+      try {
+        fromDayTimeString("5 30:12:20")
+        fail("Expected to throw an exception for the invalid input")
+      } catch {
+        case e: IllegalArgumentException =>
+          assert(e.getMessage.contains("hour 30 outside range"))
+      }
+
+      try {
+        fromDayTimeString("5 30-12")
+        fail("Expected to throw an exception for the invalid input")
+      } catch {
+        case e: IllegalArgumentException =>
+          assert(e.getMessage.contains("must match day-time format"))
+      }
+
+      try {
+        fromDayTimeString("5 1:12:20", HOUR, MICROSECOND)
+        fail("Expected to throw an exception for the invalid convention type")
+      } catch {
+        case e: IllegalArgumentException =>
+          assert(e.getMessage.contains("Cannot support (interval"))
+      }
     }
   }
 
@@ -266,4 +270,60 @@ class IntervalUtilsSuite extends SparkFunSuite {
         assert(e.getMessage.contains("divide by zero"))
     }
   }
+
+  test("from day-time string") {
+    def check(input: String, from: IntervalUnit, to: IntervalUnit, expected: String): Unit = {
+      withClue(s"from = $from, to = $to") {
+        assert(fromDayTimeString(input, from, to) === fromString(expected))
+      }
+    }
+    def checkFail(
+        input: String,
+        from: IntervalUnit,
+        to: IntervalUnit,
+        errMsg: String): Unit = {
+      try {
+        fromDayTimeString(input, from, to)
+        fail("Expected to throw an exception for the invalid input")
+      } catch {
+        case e: IllegalArgumentException =>
+          assert(e.getMessage.contains(errMsg))
+      }
+    }
+
+    check("12:40", HOUR, MINUTE, "12 hours 40 minutes")
+    check("+12:40", HOUR, MINUTE, "12 hours 40 minutes")
+    check("-12:40", HOUR, MINUTE, "-12 hours -40 minutes")
+    checkFail("5 12:40", HOUR, MINUTE, "must match day-time format")
+
+    check("12:40:30.999999999", HOUR, SECOND, "12 hours 40 minutes 30.999999 seconds")
+    check("+12:40:30.999999999", HOUR, SECOND, "12 hours 40 minutes 30.999999 seconds")
+    check("-12:40:30.999999999", HOUR, SECOND, "-12 hours -40 minutes -30.999999 seconds")
+    checkFail("5 12:40:30", HOUR, SECOND, "must match day-time format")
+    checkFail("12:40:30.0123456789", HOUR, SECOND, "must match day-time format")
+
+    check("40:30.999999999", MINUTE, SECOND, "40 minutes 30.999999 seconds")
+    check("+40:30.999999999", MINUTE, SECOND, "40 minutes 30.999999 seconds")
+    check("-40:30.999999999", MINUTE, SECOND, "-40 minutes -30.999999 seconds")
+    checkFail("12:40:30", MINUTE, SECOND, "must match day-time format")
+
+    check("5 12", DAY, HOUR, "5 days 12 hours")
+    check("+5 12", DAY, HOUR, "5 days 12 hours")
+    check("-5 12", DAY, HOUR, "-5 days -12 hours")
+    checkFail("5 12:30", DAY, HOUR, "must match day-time format")
+
+    check("5 12:40", DAY, MINUTE, "5 days 12 hours 40 minutes")
+    check("+5 12:40", DAY, MINUTE, "5 days 12 hours 40 minutes")
+    check("-5 12:40", DAY, MINUTE, "-5 days -12 hours -40 minutes")
+    checkFail("5 12", DAY, MINUTE, "must match day-time format")
+
+    check("5 12:40:30.123", DAY, SECOND, "5 days 12 hours 40 minutes 30.123 seconds")
+    check("+5 12:40:30.123456", DAY, SECOND, "5 days 12 hours 40 minutes 30.123456 seconds")
+    check("-5 12:40:30.999999999", DAY, SECOND, "-5 days -12 hours -40 minutes -30.999999 seconds")
+    checkFail("5 12", DAY, SECOND, "must match day-time format")
+
+    checkFail("5 30:12:20", DAY, SECOND, "hour 30 outside range")
+    checkFail("5 30-12", DAY, SECOND, "must match day-time format")
+    checkFail("5 1:12:20", HOUR, MICROSECOND, "Cannot support (interval")
+  }
 }

sql/core/src/test/resources/sql-tests/inputs/literals.sql

@@ -88,16 +88,21 @@ select interval 1 year 2 month 3 week 4 day 5 hour 6 minute 7 seconds 8 millisec
 select interval '30' year '25' month '-100' day '40' hour '80' minute '299.889987299' second;
 select interval '0 0:0:0.1' day to second;
 select interval '10-9' year to month;
+select interval '20 15' day to hour;
+select interval '20 15:40' day to minute;
+select interval '20 15:40:32.99899999' day to second;
+select interval '15:40' hour to minute;
+select interval '15:40:32.99899999' hour to second;
+select interval '40:32.99899999' minute to second;
+select interval '40:32' minute to second;
+-- invalid day-time string intervals
 select interval '20 15:40:32.99899999' day to hour;
 select interval '20 15:40:32.99899999' day to minute;
-select interval '20 15:40:32.99899999' day to second;
 select interval '15:40:32.99899999' hour to minute;
 select interval '15:40.99899999' hour to second;
 select interval '15:40' hour to second;
-select interval '15:40:32.99899999' hour to second;
 select interval '20 40:32.99899999' minute to second;
-select interval '40:32.99899999' minute to second;
-select interval '40:32' minute to second;
+
 -- ns is not supported
 select interval 10 nanoseconds;