Skip to content

Conversation

@otan
Copy link
Contributor

@otan otan commented Feb 25, 2020

Would resolve #44548, but has a few blockers to sort out.

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:

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.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

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.
@knz
Copy link
Contributor

knz commented Feb 26, 2020

What about taking this opportunity to get rid of lib/pq and go's sql driver.Value abstraction and instead use pgx and our own parsing into datums. There is a bunch of issues in the CLI project that would go away with this move, and it would unblock your PR.

@otan
Copy link
Contributor Author

otan commented Feb 26, 2020

Another way to go about it :) I mainly wanted to write and make sure this worked [hence the draft, but you know also a quick fix if we were bothered] because I was thinking about writing a blog post of the intricacies of dealing with time related data types, and this is an example of golang's time module being stubborn all the way down.

@otan otan closed this Mar 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lib/pq handling of 24:00:00 is broken

3 participants