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