encode: fix 2400 time encoding for time/timetz #944
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
otan
force-pushed
the
attempt_2400_fix
branch
from
0499d5e
to
bac9b53
Compare
|
looks like it passes on CI, but the "PASS" is interpreted incorrectly. |
otan
commented
Feb 25, 2020
| var is2400Time bool | ||
| switch typ { | ||
| case oid.T_timetz, oid.T_time: | ||
| if matches := time2400Regex.FindStringSubmatch(str); matches != nil { |
There was a problem hiding this comment.
(alternative parsing suggestions welcome)
otan
added a commit
to otan-cockroach/cockroach
that referenced
this pull request
Feb 25, 2020
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.
maddyblue
reviewed
May 7, 2020
otan
force-pushed
the
attempt_2400_fix
branch
2 times, most recently
from
9e77527
to
564bca4
Compare
maddyblue
reviewed
May 7, 2020
go.mod
Outdated
| @@ -1 +1,3 @@ | |||
| module github.com/lib/pq | |||
|
|
|||
| go 1.13 | |||
There was a problem hiding this comment.
running go test keeps bringing it back, urgh
Note that Postgres supports 24:00 for both time and timetz operations. When evaluating "24:00" for both Time and TimeTZ datatypes, the time.Time library does not recognise 24 as a legitimate hour. This requires special parsing for it to work. As such, work around the problem by subtracting a day, and adding it back later when we recognize it as 24:00 time.
otan
added a commit
to otan-cockroach/cockroach
that referenced
this pull request
May 21, 2020
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.
craig bot
pushed a commit
to cockroachdb/cockroach
that referenced
this pull request
May 21, 2020
49272: opt: add cost for executing geo functions in filters r=mjibson a=mjibson Geo costing is now the only function that multiplies its row count by the filters cost. The other JOINs and SELECT will be added in another PR since it is a more complicated change due to some plans changing, which must come with some justification. Fixes #48214 Release note: None 49330: pgwire: transmit 24:00 correctly for Time/TimeTZ r=rohany a=otan 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. 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 correctly emit and read in 24:00 time properly. 49372: server: Resolve default engine type in testserver r=itsbilal a=itsbilal Previously we weren't resolving the engine type if it was set to EngineTypeDefault in the testserver. This would silently resolve to Pebble (and in 20.1, rocksdb) anyway, so it was only a concern in sentry reports where it would show up as `default`. Since `cockroach demo` uses the test server and has sentry reports enabled, that was an oversight. Release note (bug fix): Report engine type correctly in bug reports when using `cockroach demo`. Co-authored-by: Matt Jibson <matt.jibson@gmail.com> Co-authored-by: Oliver Tan <otan@cockroachlabs.com> Co-authored-by: Bilal Akhtar <bilal@cockroachlabs.com>
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.
Note that Postgres supports 24:00 for both time and timetz operations.
When evaluating "24:00" for both Time and TimeTZ datatypes, the
time.Time library does not recognise 24 as a legitimate hour. This
requires special parsing for it to work. As such, work around the
problem by subtracting a day, and adding it back later when we recognize
it as 24:00 time.