[GitHub] [spark] cloud-fan commented on a change in pull request #26473: [SPARK-29864][SPARK-29920][SQL] Strict parsing of day-time strings to intervals

[GitBox <gi...@apache.org>]

cloud-fan commented on a change in pull request #26473: [SPARK-29864][SPARK-29920][SQL] Strict parsing of day-time strings to intervals
URL: https://github.com/apache/spark/pull/26473#discussion_r356458774
 
 

 ##########
 File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala
 ##########
 @@ -384,4 +388,61 @@ class IntervalUtilsSuite extends SparkFunSuite {
     val i9 = new CalendarInterval(0, 0, -3000 * MICROS_PER_HOUR)
     assert(IntervalUtils.toMultiUnitsString(i9) === "-3000 hours")
   }
+
+  test("from day-time string") {
+    def check(input: String, from: IntervalUnit, to: IntervalUnit, expected: String): Unit = {
+      withClue(s"from = $from, to = $to") {
+        val expectedUtf8 = UTF8String.fromString(expected)
+        assert(fromDayTimeString(input, from, to) === safeStringToInterval(expectedUtf8))
+      }
+    }
+    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")
 
 Review comment:
   let's avoid using `99999`. It's hard for reviewers to count the number of digits. Let's use `123456` to ease the digits counting.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org