-
Notifications
You must be signed in to change notification settings - Fork 14k
[FLINK-39995][json] Add ISO-8601 timestamp format with explicit timezone offset support #28546
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,6 +62,7 @@ | |
| import static java.time.format.DateTimeFormatter.ISO_LOCAL_DATE; | ||
| import static org.apache.flink.formats.common.TimeFormats.ISO8601_TIMESTAMP_FORMAT; | ||
| import static org.apache.flink.formats.common.TimeFormats.ISO8601_TIMESTAMP_WITH_LOCAL_TIMEZONE_FORMAT; | ||
| import static org.apache.flink.formats.common.TimeFormats.ISO8601_TIMESTAMP_WITH_OFFSET_FORMAT; | ||
| import static org.apache.flink.formats.common.TimeFormats.SQL_TIMESTAMP_FORMAT; | ||
| import static org.apache.flink.formats.common.TimeFormats.SQL_TIMESTAMP_WITH_LOCAL_TIMEZONE_FORMAT; | ||
| import static org.apache.flink.formats.common.TimeFormats.SQL_TIME_FORMAT; | ||
|
|
@@ -278,6 +279,17 @@ private TimestampData convertToTimestamp(JsonParser jp) throws IOException { | |
| case ISO_8601: | ||
| parsedTimestamp = ISO8601_TIMESTAMP_FORMAT.parse(jp.getText()); | ||
| break; | ||
| case ISO_8601_WITH_OFFSET: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. my AI says For Half-Hour Timezones: Add comprehensive tests: root.put("timestamp_india", "1990-10-14T12:12:43.123+05:30"); Consider: Whether output should preserve the original offset instead of always converting to UTC. Current behavior may surprise users expecting offset preservation. Documentation: Clearly state that this format converts all timestamps to UTC offset (+00:00) on output, regardless of input offset. |
||
| parsedTimestamp = ISO8601_TIMESTAMP_WITH_OFFSET_FORMAT.parse(jp.getText()); | ||
| ZoneOffset offset = parsedTimestamp.query(TemporalQueries.offset()); | ||
| if (offset != null) { | ||
| return TimestampData.fromInstant( | ||
| LocalDateTime.of( | ||
| parsedTimestamp.query(TemporalQueries.localDate()), | ||
| parsedTimestamp.query(TemporalQueries.localTime())) | ||
| .toInstant(offset)); | ||
| } | ||
| break; | ||
| default: | ||
| throw new TableException( | ||
| String.format( | ||
|
|
@@ -301,6 +313,10 @@ private TimestampData convertToTimestampWithLocalZone(JsonParser jp) throws IOEx | |
| parsedTimestampWithLocalZone = | ||
| ISO8601_TIMESTAMP_WITH_LOCAL_TIMEZONE_FORMAT.parse(jp.getText()); | ||
| break; | ||
| case ISO_8601_WITH_OFFSET: | ||
| parsedTimestampWithLocalZone = | ||
| ISO8601_TIMESTAMP_WITH_OFFSET_FORMAT.parse(jp.getText()); | ||
| break; | ||
| default: | ||
| throw new TableException( | ||
| String.format( | ||
|
|
@@ -309,9 +325,11 @@ private TimestampData convertToTimestampWithLocalZone(JsonParser jp) throws IOEx | |
| } | ||
| LocalTime localTime = parsedTimestampWithLocalZone.query(TemporalQueries.localTime()); | ||
| LocalDate localDate = parsedTimestampWithLocalZone.query(TemporalQueries.localDate()); | ||
| ZoneOffset offset = parsedTimestampWithLocalZone.query(TemporalQueries.offset()); | ||
|
|
||
| return TimestampData.fromInstant( | ||
| LocalDateTime.of(localDate, localTime).toInstant(ZoneOffset.UTC)); | ||
| LocalDateTime.of(localDate, localTime) | ||
| .toInstant(offset != null ? offset : ZoneOffset.UTC)); | ||
| } | ||
|
|
||
| private StringData convertToString(JsonParser jp) throws IOException { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please could you update the docs around this change.