One formatter for Timestamp fields in Storage writes #25472
Conversation
There was a problem hiding this comment.
we should not catch an exception to determine we should use an alternate date time formatter
There was a problem hiding this comment.
that was already the current implementation, will change to have one date time formatter to parse from.
ahmedabu98
force-pushed
the
utc_timestamp
branch
from
64b4a46 to
1627b0a
Compare
ahmedabu98
changed the title
|
R: @johnjcasey PTAL |
| .appendLiteral(' ') | ||
| .append(DateTimeFormatter.ISO_LOCAL_TIME) | ||
| .optionalStart() | ||
| .appendLiteral(" UTC") |
There was a problem hiding this comment.
I think we should support any time zone here, not just UTC.
Even better, support all formats listed in BigQuery documentation, i.e.:
- 2014-09-27T12:30:00.45Z
- 2014-09-27 12:30:00.45-8:00
- 2014-09-27 12:30:00.123456 UTC
- 2014-09-27 12:30:00.123456 America/Los_Angeles
Note that in all 4 formats BQ accepts either space or "T" as the date/time separator. And as for the datetime/time zone separator: for "Z" and "-8:00" time zone format there must be no space before the time zone, and for "UTC" and "America/Los_Angeles" format there must be a space before.
| .optionalStart() | ||
| .appendLiteral(' ') | ||
| .optionalEnd() | ||
| .optionalStart() | ||
| .appendLiteral('T') | ||
| .optionalEnd() |
There was a problem hiding this comment.
FYI this allows for timestamp fields in TableRows to look like this:
2000-02-13T12:30:002000-02-13 12:30:002000-02-13 T12:30:002000-02-1312:30:00
All of these would be handled the same way so I don't think there is much concern. If there's an issue however, I can split up the DATETIME_SPACE_FORMATTER so only 1) and 2) are valid
|
@johnjcasey, @an2x ready for review |
|
LGTM |
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
merge HEAD to pull config fixes
|
Assigning reviewers. If you would like to opt out of this review, comment R: @apilloud for label java. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
R: @johnjcasey @an2x |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
* handle timestamps with one formatter; handle UTC suffix * timestamp formatter that handles zone region suffix
4 participants
Allow timestamp fields with UTC suffix
e.g. "2023-02-14 12:00:00.123 UTC"
Also, change to use one date time formatter