Skip to content
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

[NH] Support timetz datatype #5964

Closed
jseldess opened this issue Nov 26, 2019 · 8 comments · Fixed by #6391
Closed

[NH] Support timetz datatype #5964

jseldess opened this issue Nov 26, 2019 · 8 comments · Fixed by #6391
Assignees
Labels
P-0 Urgent; must be done in next 2 weeks
Milestone

Comments

@jseldess
Copy link
Contributor

jseldess commented Nov 26, 2019

Background: https://airtable.com/tblD3oZPLJgGhCmch/viw1DKmbKhg2MIECH/recgAULbWY1898miw

Description: CockroachDB supports a new data type timetz that includes both the time of day and time zone.

Postgres alias time (time [ (p) ] with time zone) with timetz to include both the time of day and time zone. Many third-party tools like Hibernate expect this data type to include time zone information.

Team: Andrew Woods, Jordan Lewis

Github Tracking Issue: cockroachdb/cockroach#26097 cockroachdb/cockroach#20944 cockroachdb/cockroach#16490 #2982 #5964

@jseldess jseldess added this to the 20.1 milestone Nov 26, 2019
@ericharmeling ericharmeling added the P-0 Urgent; must be done in next 2 weeks label Jan 9, 2020
@ericharmeling
Copy link
Contributor

@awoods187

We had docs for this, then removed them, to discourage users from using TIMETZ (see #3681).
What changed here that we are reinstating TIMETZ? Are there still problems present to warn users about?

@ericharmeling
Copy link
Contributor

Relevant PR: cockroachdb/cockroach#43023

@awoods187
Copy link
Contributor

@otan can give you the details but we addressed all known bugs in our TIMETZ implementation with this work and we should definitely document it.

@otan
Copy link
Contributor

otan commented Jan 13, 2020

Yep, it should basically work exactly how postgres does it now!

Only docs callouts (based on cockroachdb/cockroach#43023)

  • Note that it has 12 bytes of information instead of 8.
  • I would also add a warning that TimeTZ displays a date of 0000-01-01 (as does TIME)

@ericharmeling
Copy link
Contributor

Yep, it should basically work exactly how postgres does it now!

Only docs callouts (based on cockroachdb/cockroach#43023)

  • Note that it has 12 bytes of information instead of 8.
  • I would also add a warning that TimeTZ displays a date of 0000-01-01 (as does TIME)

Sweet. Thanks @otan!

@otan
Copy link
Contributor

otan commented Jan 13, 2020

Oh, and the main gotcha is that Time will display time based on context timezone (e.g. SET TIME ZONE +3 will display all TIme in +3), but TimeTZ will display time based on the timezone that was set on the data type (e.g. if you set +5 in the TimeTZ, SET TIME ZONE -5 will still display +5). Ordering is done in terms of "epoch", with negative timezones being ranked "higher" if times are equal, e.g. '2:00 -1' > '2:00 +0', '12:00-1' > '1:00+0')

If Postgres docs doesn't cover this kind of subtlety, let me know and I can be a bit more detailed.

@otan
Copy link
Contributor

otan commented Jan 16, 2020

on research, we should actually discourage timetz: https://dba.stackexchange.com/questions/187351/what-is-a-valid-use-case-for-using-time-with-time-zone/187359#187359

@jseldess
Copy link
Contributor Author

jseldess commented Feb 5, 2020

From release notes:

  • CockroachDB now allows the usage of TIMETZ throughout the cluster. [#43023][#43023]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-0 Urgent; must be done in next 2 weeks
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants