[GitHub] [spark] MaxGekk commented on a change in pull request #26079: [SPARK-29369][SQL] Support string intervals without the `interval` prefix

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

MaxGekk commented on a change in pull request #26079: [SPARK-29369][SQL] Support string intervals without the `interval` prefix
URL: https://github.com/apache/spark/pull/26079#discussion_r333469680
 
 

 ##########
 File path: common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java
 ##########
 @@ -73,45 +72,50 @@ private static long toLong(String s) {
    * This method is case-insensitive.
    */
   public static CalendarInterval fromString(String s) {
-    if (s == null) {
-      return null;
-    }
-    s = s.trim();
-    Matcher m = p.matcher(s);
-    if (!m.matches() || s.compareToIgnoreCase("interval") == 0) {
+    try {
+      return fromCaseInsensitiveString(s);
+    } catch (IllegalArgumentException e) {
       return null;
-    } else {
-      long months = toLong(m.group(1)) * 12 + toLong(m.group(2));
-      long microseconds = toLong(m.group(3)) * MICROS_PER_WEEK;
-      microseconds += toLong(m.group(4)) * MICROS_PER_DAY;
-      microseconds += toLong(m.group(5)) * MICROS_PER_HOUR;
-      microseconds += toLong(m.group(6)) * MICROS_PER_MINUTE;
-      microseconds += toLong(m.group(7)) * MICROS_PER_SECOND;
-      microseconds += toLong(m.group(8)) * MICROS_PER_MILLI;
-      microseconds += toLong(m.group(9));
-      return new CalendarInterval((int) months, microseconds);
     }
   }
 
   /**
-   * Convert a string to CalendarInterval. Unlike fromString, this method can handle
+   * Convert a string to CalendarInterval. This method can handle
    * strings without the `interval` prefix and throws IllegalArgumentException
    * when the input string is not a valid interval.
    *
    * @throws IllegalArgumentException if the string is not a valid internal.
    */
   public static CalendarInterval fromCaseInsensitiveString(String s) {
 
 Review comment:
   antlr parser does this as well but [it parses sql elements](https://github.com/apache/spark/blob/f2ead4d0b50715e3dec79ce762c31f145d46a5b5/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala#L1907-L1961) like
   ```sql
   spark-sql> select interval 10 days 1 second;
   interval 1 weeks 3 days 1 seconds
   ```
   here is only the place where we parse string values:
   ```sql
   spark-sql> select interval 'interval 10 days 1 second';
   interval 1 weeks 3 days 1 seconds
   ```

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