[GitHub] [flink] fsk119 commented on a change in pull request #12756: [FLINK-18296][json] add support for TIMESTAMP_WITH_LOCAL_ZONE and fix…

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

fsk119 commented on a change in pull request #12756:
URL: https://github.com/apache/flink/pull/12756#discussion_r449538389



##########
File path: flink-formats/flink-json/src/main/java/org/apache/flink/formats/json/TimeFormats.java
##########
@@ -31,7 +31,7 @@
 	static final DateTimeFormatter RFC3339_TIME_FORMAT = new DateTimeFormatterBuilder()
 		.appendPattern("HH:mm:ss")
 		.appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true)
-		.appendPattern("'Z'")
+		.appendPattern("X")

Review comment:
       The use of 'X' allow users to input data with different timezone. The reasons follow:
   1) it's much flexiable for users if their input timestamp contains timezone. Use 'X' we can parse data with different timezone such as Asia/Shanghai without explicitly type conversion;
   2) I think the standard above is about timestamp not timestamp with local zone. Oracle offers an example of TIMESTAMP WITH TIMEZONE in link[1].
   3) RFC3339 has the ability to parse timestamp with different timezone. In origin implementation, the input must with UTC timezone which is not as same as RFC3339 standard.
   
   [1] https://docs.oracle.com/cd/B19306_01/server.102/b14225/ch4datetime.htm#i1006081
   
    




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