cloud-fan commented on a change in pull request #26256: [SPARK-29605][SQL] 
Optimize string to interval casting
URL: https://github.com/apache/spark/pull/26256#discussion_r343102488
 
 

 ##########
 File path: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala
 ##########
 @@ -37,58 +65,41 @@ class IntervalUtilsSuite extends SparkFunSuite {
     testSingleUnit("MilliSecond", 3, 0, 0, 3 * MICROS_PER_MILLIS)
     testSingleUnit("MicroSecond", 3, 0, 0, 3)
 
-    for (input <- Seq(null, "", " ")) {
-      try {
-        fromString(input)
-        fail("Expected to throw an exception for the invalid input")
-      } catch {
-        case e: IllegalArgumentException =>
-          val msg = e.getMessage
-          if (input == null) {
-            assert(msg.contains("cannot be null"))
-          }
-      }
-    }
+    checkFromInvalidString(null, "cannot be null")
 
-    for (input <- Seq("interval", "interval1 day", "foo", "foo 1 day")) {
-      try {
-        fromString(input)
-        fail("Expected to throw an exception for the invalid input")
-      } catch {
-        case e: IllegalArgumentException =>
-          val msg = e.getMessage
-          assert(msg.contains("Invalid interval string"))
-      }
+    for (input <- Seq("", " ", "interval", "interval1 day", "foo", "foo 1 
day")) {
+      checkFromInvalidString(input, "Invalid interval string")
     }
   }
 
-  test("fromString: random order field") {
-    val input = "1 day 1 year"
-    val result = new CalendarInterval(12, 1, 0)
-    assert(fromString(input) == result)
-  }
 
-  test("fromString: duplicated fields") {
-    val input = "1 day 1 day"
-    val result = new CalendarInterval(0, 2, 0)
-    assert(fromString(input) == result)
+  test("string to interval: multiple units") {
+    Seq(
+      "-1 MONTH 1 day -1 microseconds" -> new CalendarInterval(-1, 1, -1),
+      " 123 MONTHS        123 DAYS  123 Microsecond    " -> new 
CalendarInterval(123, 123, 123),
+      "interval -1 day +3 Microseconds" -> new CalendarInterval(0, -1, 3),
+      "  interval  8  years -11 months 123  weeks   -1 day " +
+        "23 hours -22 minutes 1 second  -123  millisecond    567 microseconds 
" ->
+        new CalendarInterval(85, 860, 81480877567L)).foreach { case (input, 
expected) =>
+      checkFromString(input, expected)
+    }
   }
 
-  test("fromString: value with +/-") {
-    val input = "+1 year -1 day"
-    val result = new CalendarInterval(12, -1, 0)
-    assert(fromString(input) == result)
+  test("string to interval: special cases") {
 
 Review comment:
   can we also test `1. second`, `1. hour` and `1.111111111 seconds` here?

----------------------------------------------------------------
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

Reply via email to