[ 
https://issues.apache.org/jira/browse/HUDI-1939?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17382771#comment-17382771
 ] 

ASF GitHub Bot commented on HUDI-1939:
--------------------------------------

xushiyan commented on a change in pull request #3003:
URL: https://github.com/apache/hudi/pull/3003#discussion_r671803228



##########
File path: 
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/keygen/parser/HoodieDateTimeParserImpl.java
##########
@@ -51,22 +47,21 @@ private DateTimeFormatter getInputDateFormatter() {
       throw new 
IllegalArgumentException(Config.TIMESTAMP_INPUT_DATE_FORMAT_PROP + " 
configuration is required");
     }
 
-    DateTimeFormatter formatter = new DateTimeFormatterBuilder()
-        .append(
-        null,
-        Arrays.stream(
-          
this.configInputDateFormatList.split(super.configInputDateFormatDelimiter))
-          .map(String::trim)
-          .map(DateTimeFormat::forPattern)
-          .map(DateTimeFormatter::getParser)
-          .toArray(DateTimeParser[]::new))
+    DateTimeFormatterBuilder formatterBuilder = new DateTimeFormatterBuilder();
+    
Arrays.stream(this.configInputDateFormatList.split(super.configInputDateFormatDelimiter))
+        .map(String::trim)
+        .map(DateTimeFormatter::ofPattern)
+        .forEach(formatterBuilder::appendOptional);
+    DateTimeFormatter formatter = formatterBuilder
+        .parseDefaulting(ChronoField.HOUR_OF_DAY, 0)
+        .parseDefaulting(ChronoField.MINUTE_OF_HOUR, 0)
+        .parseDefaulting(ChronoField.SECOND_OF_MINUTE, 0)
         .toFormatter();
     if (this.inputDateTimeZone != null) {
       formatter = formatter.withZone(this.inputDateTimeZone);
     } else {
-      formatter = formatter.withOffsetParsed();
+      formatter = formatter.withZone(ZoneId.systemDefault());

Review comment:
       this is not fully compatible change: joda has a feature to infer 
timezone from input string but java time does not. so it supplies system 
default timezone.

##########
File path: 
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/keygen/TestTimestampBasedKeyGenerator.java
##########
@@ -223,9 +223,9 @@ public void 
test_ExpectsMatch_SingleInputFormat_ISO8601WithMsZ_OutputTimezoneAsU
     baseRecord.put("createTime", "2020-04-01T13:01:33.428Z");
     properties = this.getBaseKeyConfig(
         "DATE_STRING",
-        "yyyy-MM-dd'T'HH:mm:ss.SSSZ",
-        "",
+        "yyyy-MM-dd'T'HH:mm:ss.SSSX",
         "",
+        "GMT",

Review comment:
       as commented above, since using java time will by default give system 
default timezone, we need to set the timezone specifically to make sure it 
matches with input data's timezone `Z`

##########
File path: 
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/keygen/TestTimestampBasedKeyGenerator.java
##########
@@ -331,11 +331,11 @@ public void 
test_ExpectsMatch_MultipleInputFormats_ISO8601WithMsZ_OutputTimezone
     baseRecord.put("createTime", "2020-04-01T13:01:33.123Z");
     properties = this.getBaseKeyConfig(
         "DATE_STRING",
-        "yyyy-MM-dd'T'HH:mm:ssZ,yyyy-MM-dd'T'HH:mm:ss.SSSZ",
-        "",
+        "yyyy-MM-dd'T'HH:mm:ssX,yyyy-MM-dd'T'HH:mm:ss.SSSX",
         "",
+        "GMT",
         "yyyyMMddHH",
-        "EST");
+        "EST5EDT");

Review comment:
       timezone `EST` has daylight savings issue; use `EST5EDT` to handle it.




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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


> Replace joda-time api to new jdk8 time api
> ------------------------------------------
>
>                 Key: HUDI-1939
>                 URL: https://issues.apache.org/jira/browse/HUDI-1939
>             Project: Apache Hudi
>          Issue Type: Improvement
>            Reporter: Xuedong Luan
>            Assignee: Xuedong Luan
>            Priority: Minor
>              Labels: pull-request-available
>             Fix For: 0.9.0
>
>
> remove the usage of joda-time api and use the new jdk8 time api



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to