You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by jiang-wu <gi...@git.apache.org> on 2018/05/01 23:08:00 UTC

[GitHub] drill pull request #1247: DRILL-6242 Use java.time.Local{Date|Time|DateTime}...

GitHub user jiang-wu opened a pull request:

    https://github.com/apache/drill/pull/1247

    DRILL-6242 Use java.time.Local{Date|Time|DateTime} for Drill Date, Time, and Timestamp types

    * DRILL-6242 - Use java.time.Local{Date|Time|DateTime} classes to hold values from corresponding Drill date, time, and timestamp types.
    * See https://issues.apache.org/jira/browse/DRILL-6242?focusedCommentId=16459369&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16459369
    * This is a revised version of https://github.com/apache/drill/pull/1184

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jiang-wu/drill DRILL-6242-LocalDateTime

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/drill/pull/1247.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1247
    
----
commit b00638da507e6211d57c9ea7d6308f323aad9519
Author: jiang-wu <jw...@...>
Date:   2018-05-01T21:48:26Z

    DRILL-6242 Use java.time.Local{Date|Time|DateTime} for Drill Date, Time, Timestamp types. (#3)
    
    * DRILL-6242 - Use java.time.Local{Date|Time|DateTime} classes to hold values from corresponding Drill date, time, and timestamp types.

----


---

[GitHub] drill issue #1247: DRILL-6242 Use java.time.Local{Date|Time|DateTime} for Dr...

Posted by jiang-wu <gi...@git.apache.org>.
Github user jiang-wu commented on the issue:

    https://github.com/apache/drill/pull/1247
  
    Please see https://issues.apache.org/jira/browse/DRILL-6242?focusedCommentId=16459369&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16459369 on the results of this change.  The behavior is the same as the current Drill behavior, except returning Local{Date|Time|DateTime} upon reading from the vectors.
    
    Notice the differences in Drill behavior in handling the date time data from different data sources.  We can separately decide how to make those consistent.  Fixing those differences are out of scope for this pull request.


---

[GitHub] drill issue #1247: DRILL-6242 Use java.time.Local{Date|Time|DateTime} for Dr...

Posted by jiang-wu <gi...@git.apache.org>.
Github user jiang-wu commented on the issue:

    https://github.com/apache/drill/pull/1247
  
    @parthchandra @vdiravka A new pull request that uses java.time.Local{Date|Time|DateTime}.


---

[GitHub] drill pull request #1247: DRILL-6242 Use java.time.Local{Date|Time|DateTime}...

Posted by jiang-wu <gi...@git.apache.org>.
Github user jiang-wu commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1247#discussion_r185358628
  
    --- Diff: exec/vector/src/main/java/org/apache/drill/exec/expr/fn/impl/DateUtility.java ---
    @@ -639,29 +648,95 @@ public static String getTimeZone(int index) {
         return timezoneList[index];
       }
     
    +  /**
    +   * Parse given string into a LocalDate
    +   */
    +  public static LocalDate parseLocalDate(final String value) {
    +      return LocalDate.parse(value, formatDate);
    +  }
    +
    +  /**
    +   * Parse given string into a LocalTime
    +   */
    +  public static LocalTime parseLocalTime(final String value) {
    +      return LocalTime.parse(value, formatTime);
    +  }
    +
    +  /**
    +   * Parse the given string into a LocalDateTime.
    +   */
    +  public static LocalDateTime parseLocalDateTime(final String value) {
    +      return LocalDateTime.parse(value, formatTimeStamp);
    +  }
    +
       // Returns the date time formatter used to parse date strings
       public static DateTimeFormatter getDateTimeFormatter() {
     
         if (dateTimeTZFormat == null) {
    -      DateTimeFormatter dateFormatter = DateTimeFormat.forPattern("yyyy-MM-dd");
    -      DateTimeParser optionalTime = DateTimeFormat.forPattern(" HH:mm:ss").getParser();
    -      DateTimeParser optionalSec = DateTimeFormat.forPattern(".SSS").getParser();
    -      DateTimeParser optionalZone = DateTimeFormat.forPattern(" ZZZ").getParser();
    +      DateTimeFormatter dateFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd");
    +      DateTimeFormatter optionalTime = DateTimeFormatter.ofPattern(" HH:mm:ss");
    +      DateTimeFormatter optionalSec = DateTimeFormatter.ofPattern(".SSS");
    +      DateTimeFormatter optionalZone = DateTimeFormatter.ofPattern(" ZZZ");
     
    -      dateTimeTZFormat = new DateTimeFormatterBuilder().append(dateFormatter).appendOptional(optionalTime).appendOptional(optionalSec).appendOptional(optionalZone).toFormatter();
    +      dateTimeTZFormat = new DateTimeFormatterBuilder().parseLenient()
    +              .append(dateFormatter)
    +              .appendOptional(optionalTime)
    +              .appendOptional(optionalSec)
    +              .appendOptional(optionalZone)
    +              .toFormatter();
         }
     
         return dateTimeTZFormat;
       }
     
    +  /**
    --- End diff --
    
    parseBest is used only by JUnit tests when the string value is not very strict.  Example, "2018-1-1 12:1" instead of "2018-01-01 12:01".  This method is more lenient and tolerating missing parts when parsing a date time. 


---

[GitHub] drill pull request #1247: DRILL-6242 Use java.time.Local{Date|Time|DateTime}...

Posted by jiang-wu <gi...@git.apache.org>.
Github user jiang-wu commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1247#discussion_r185358343
  
    --- Diff: exec/vector/src/main/java/org/apache/drill/exec/expr/fn/impl/DateUtility.java ---
    @@ -639,29 +648,95 @@ public static String getTimeZone(int index) {
         return timezoneList[index];
       }
     
    +  /**
    --- End diff --
    
    The "parseLocalDate", "parseLocalTime", "parseLocalDateTime" are used by various junit tests.  These parsers are strict in that if the input string doesn't have all the specified fields, it will fail to parse.


---