sql: add precision support for TIME/TIMETZ #42668
Merged
Conversation
This file contains 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
|
This change is |
otan
changed the title
otan
force-pushed
the
otan-precision_time
branch
3 times, most recently
from
374d633
to
1244ba5
Compare
otan
force-pushed
the
otan-precision_time
branch
from
1244ba5
to
f769516
Compare
solongordon
approved these changes
Dec 2, 2019
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @solongordon)
pkg/util/timeofday/time_of_day.go, line 106 at r1 (raw file):
} ret := t.ToTime().Round(precision) // Rounding Max should give Time2400, not 00:00.
Good catch!
pkg/util/timeofday/time_of_day.go, line 108 at r1 (raw file):
// Rounding Max should give Time2400, not 00:00. // To catch this, see if we went to the next day. if ret.Day() == 2 {
Nit: Could make this ret.Day() != t.ToTime().Day() to get rid of the magical constant and make it less sensitive to the underlying ToTime() behavior.
otan
force-pushed
the
otan-precision_time
branch
from
f769516
to
9100c31
Compare
Adding support for precision for both TIME and TIMETZ. This also includes threading through some extra parsing syntax for TIMETZ. ALTER TABLE between TIME and TIMETZ not supported as they have different representations. Release note (sql change): This PR adds new support for precision for TIME types (e.g. TIME(3) will truncate to milliseconds). Previously this would raise syntax errors.
otan
force-pushed
the
otan-precision_time
branch
from
9100c31
to
2c3ade7
Compare
|
bors r+ |
craig bot
pushed a commit
that referenced
this pull request
Dec 3, 2019
42668: sql: add precision support for TIME/TIMETZ r=otan a=otan Refs: #32565 Refs: #26097 Adding support for precision for both TIME and TIMETZ. This also includes threading through some extra parsing syntax for TIMETZ. ALTER TABLE between TIME and TIMETZ not supported as they have different representations. Release note (sql change): This PR adds new support for precision for TIME types (e.g. TIME(3) will truncate to milliseconds). Previously this would raise syntax errors. 42864: rfcs: new RFC on fixing the halloween problem r=knz a=knz I am splitting this RFC away from the [RFC on savepoints](#41569) because I know recognize it is a different problem with an orthogonal solution. https://github.com/knz/cockroach/blob/20191129-rfc-halloween/docs/RFCS/20191014_halloween.md There is no new idea in this text (I copy-pasted the text from the other RFC) and the implementation PRs are already out, so if the reviewers are satisfied I would gladly already merge the RFC as in-progress or completed. Co-authored-by: Oliver Tan <otan@cockroachlabs.com> Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Refs: #32565
Refs: #26097
Adding support for precision for both TIME and TIMETZ. This also
includes threading through some extra parsing syntax for TIMETZ.
ALTER TABLE between TIME and TIMETZ not supported as they have different
representations.
Release note (sql change): This PR adds new support for precision for
TIME types (e.g. TIME(3) will truncate to milliseconds). Previously this
would raise syntax errors.