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

Audit and fix 24:00 handling for Time/TimeTZ #44530

Closed
otan opened this issue Jan 30, 2020 · 3 comments
Closed

Audit and fix 24:00 handling for Time/TimeTZ #44530

otan opened this issue Jan 30, 2020 · 3 comments
Assignees
Labels
A-sql-semantics C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@otan
Copy link
Contributor

otan commented Jan 30, 2020

Postgres has a unique max time of 24:00:00 for both TimeTZ and Time.

They can be stored/parsed as 24:00:00:


otan=# select '24:00'::time;
   time
----------
 24:00:00
(1 row)

otan=# select '24:00:00'::timetz;
   timetz
-------------
 24:00:00-08
(1 row)

But not "added" to '24:00:00':

otan=# select '23:59'::time + '1min'::interval;
 ?column?
----------
 00:00:00
(1 row)

Our timeofday library for parsing time/timetz (timeofday.New) or inferring time (timeofday.FromInt) always assumes that we should round -- but there are cases (such as cli/dump.go) where we should actually NOT round when doing this.

We should audit and fix all the cases where rounding is not desired.

This was caught "by accident" in #44459.

@otan otan added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-sql-typing SQLtype inference, typing rules, type compatibility. labels Jan 30, 2020
@otan otan self-assigned this Jan 30, 2020
@otan otan changed the title Audit and fix 24:00 handling for Time/TimeTZtimeofday.New|FromInt Audit and fix 24:00 handling for Time/TimeTZ` Jan 30, 2020
@otan otan added A-sql-semantics and removed A-sql-typing SQLtype inference, typing rules, type compatibility. labels Jan 30, 2020
@otan otan changed the title Audit and fix 24:00 handling for Time/TimeTZ` Audit and fix 24:00 handling for Time/TimeTZ Jan 30, 2020
@otan
Copy link
Contributor Author

otan commented Jan 31, 2020

turns out to not be a problem.

@otan otan closed this as completed Jan 31, 2020
@awoods187
Copy link
Contributor

Could you expand upon this further @otan ?

@otan
Copy link
Contributor Author

otan commented Feb 1, 2020

The real problem is #44548

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-semantics C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

No branches or pull requests

2 participants