You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/03/10 08:39:40 UTC

[GitHub] [spark] yaooqinn commented on a change in pull request #26815: [SPARK-30189][SQL] Interval from year-month/date-time string should handle whitespaces

yaooqinn commented on a change in pull request #26815: [SPARK-30189][SQL] Interval from year-month/date-time string should handle whitespaces
URL: https://github.com/apache/spark/pull/26815#discussion_r390160079
 
 

 ##########
 File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala
 ##########
 @@ -312,6 +321,11 @@ class IntervalUtilsSuite extends SparkFunSuite with SQLHelper {
     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")
+
+    // whitespaces
+    check("\t +5 12:40\t ", DAY, MINUTE, "5 days 12 hours 40 minutes")
+    checkFail("+5\t 12:40", DAY, MINUTE, "must match day-time format")
 
 Review comment:
   yes. My opinion is that the interval string in multi-unit syntax, we parse each value and each unit separately and they can be produced by the providers separately too so the whitespaces can be in the middle of each part. In year-month and day-time syntax, the string should be considered as a whole part so the whitespaces only appear on both ends, not in the middle.
   
   take java.sql.Timestamp for an example, the timestamp string is treated as a whole
   
   ```
   scala> java.sql.Timestamp.valueOf("2011-11-11 11:11:11.11\t")
   res8: java.sql.Timestamp = 2011-11-11 11:11:11.11
   
   scala> java.sql.Timestamp.valueOf("2011-11-11 \t 11:11:11.11\t")
   java.lang.NumberFormatException: For input string: "	 11"
     at java.lang.NumberFormatException.forInputString(NumberFormatException.java:65)
     at java.lang.Integer.parseInt(Integer.java:569)
     at java.lang.Integer.parseInt(Integer.java:615)
     at java.sql.Timestamp.valueOf(Timestamp.java:243)
     ... 28 elided
   ```
   
   Also, we do the same in Spark
   
   ```
   spark-sql> select timestamp '2013-01-02 \t 00:00:00';
   Error in query:
   Cannot parse the TIMESTAMP value: 2013-01-02 	 00:00:00(line 1, pos 7)
   
   == SQL ==
   select timestamp '2013-01-02 \t 00:00:00'
   -------^^^
   
   spark-sql> select timestamp '2013-01-02 00:00:00 \t';
   2013-01-02 00:00:00
   ```

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