pgwire: transmit 24:00 correctly for Time/TimeTZ #49330
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
otan
requested review from
a team,
pbardea and
rohany
and removed request for
a team
Member
|
This change is |
Contributor
|
Whats the reason for adding a new function here that allows 24:00? Why can't we just update existing cases to handle it? |
Contributor
Author
|
Because existing functions will overflow 24:00 to 00:00. This needs to be explicit and that's only from parsing and reading from disk. |
rohany
approved these changes
May 21, 2020
Contributor
rohany
left a comment
rohany
left a comment
There was a problem hiding this comment.
LGTM, i would just make that more explicit then -- I can imagine a month or two from now I wouldn't remember which one to use where.
We previously relied on golang's time.Time library to return 24:00 time for us; however, time.Time does not parse 24:00 well, nor does it print 24:00 as on the same day (it uses the next day). As such, when parsing or emitting time.Time, we have to be super weary of how we output 24:00 time. Two major changes: * update lib/pq to pick up lib/pq#944 such that it can parse 24:00 time externally. * update places where time.Time is parsed from a go driver or lib/pq to correctly handle 24:00 cases. Release note (sql change, bug fix): 24:00 time now displays correctly in the cli, returning `0001-01-02 00:00:00`. Furthermore, backups correctly emit and read in 24:00 time properly.
Contributor
Author
|
tftr! bors r=rohany |
Contributor
Build failed (retrying...) |
Contributor
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We previously relied on golang's time.Time library to return 24:00 time
for us; however, time.Time does not parse 24:00 well, nor does it print
24:00 as on the same day (it uses the next day).
As such, when parsing or emitting time.Time, we have to be super weary
of how we output 24:00 time. Two major changes:
it can parse 24:00 time externally.
correctly handle 24:00 cases.
Resolves #44548.
Release note (sql change, bug fix): 24:00 time now displays correctly in
the cli, returning
0001-01-02 00:00:00. Furthermore, backups correctlyemit and read in 24:00 time properly.