sql: fix parsing of 0000-01-01 as Time/TimeTZ #42762
Conversation
|
This change is |
otan
force-pushed
the
otan-parse_time
branch
3 times, most recently
from
b6d8728 to
16ab75a
Compare
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @otan)
pkg/sql/logictest/testdata/logic_test/time, line 350 at r1 (raw file):
query T SELECT '0000-01-01 24:00:00'::time
Maybe worth casting it to string to check that it's actually 24:00 under the hood?
pkg/sql/sem/tree/datum.go, line 1876 at r1 (raw file):
// Replace "0000-01-01" as a date (which lib/pq outputs) to // "1970-01-01", as pgdate cannot parse 0000-01-01. s = strings.ReplaceAll(s, "0000-01-01", "1970-01-01")
Minor: Perhaps cheaper to only check for 0000-01-01 at the beginning of the string.
pkg/util/timetz/timetz.go, line 42 at r1 (raw file):
// timeTZHasTimeComponent determines whether there is a time component at all // in a given string. timeTZHasTimeComponent = regexp.MustCompile(`\d+:`)
Nit: This is equivalent to \d:, no?
pkg/util/timetz/timetz.go, line 104 at r1 (raw file):
// Replace "0000-01-01" as a date (which lib/pq outputs) to // "1970-01-01", as pgdate cannot parse 0000-01-01. s = strings.ReplaceAll(s, "0000-01-01", "1970-01-01")
Again seems unnecessary to replace at other points besides the beginning of the string.
There was a problem hiding this comment.
Reviewable status:
pkg/sql/logictest/testdata/logic_test/time, line 350 at r1 (raw file):
Previously, solongordon (Solon) wrote…
Maybe worth casting it to
stringto check that it's actually24:00under the hood?
Done.
pkg/sql/sem/tree/datum.go, line 1876 at r1 (raw file):
Previously, solongordon (Solon) wrote…
Minor: Perhaps cheaper to only check for
0000-01-01at the beginning of the string.
Done!
pkg/util/timetz/timetz.go, line 104 at r1 (raw file):
Previously, solongordon (Solon) wrote…
Again seems unnecessary to replace at other points besides the beginning of the string.
Done.
otan
force-pushed
the
otan-parse_time
branch
6 times, most recently
from
61e2a34 to
1cb2069
Compare
|
bors r+ |
Merge conflict |
Since `lib/pq` outputs `time.Time` for time datums as 0000-01-01, we should be able to parse this in. However, `pgdate` library cannot do that - so we hack around it for now by replacing the year for these kinds of dates. Release note (bug fix): Previously, attempting to parse `0000-01-01 00:00` when involving `time` did not work as `pgdate` does not understand `0000` as a year. This PR will fix that behaviour.
|
bors r+ |
42762: sql: fix parsing of 0000-01-01 as Time/TimeTZ r=otan a=otan Resolves #42749 Since `lib/pq` outputs `time.Time` for time datums as 0000-01-01, we should be able to parse this in. However, `pgdate` library cannot do that - so we hack around it for now by replacing the year for these kinds of dates. Release note (bug fix): Previously, attempting to parse `0000-01-01 00:00` when involving `time` did not work as `pgdate` does not understand `0000` as a year. This PR will fix that behaviour. Co-authored-by: Oliver Tan <[email protected]>
Build succeeded |
Resolves #42749
Since
lib/pqoutputstime.Timefor time datums as 0000-01-01, weshould be able to parse this in. However,
pgdatelibrary cannot dothat - so we hack around it for now by replacing the year for these
kinds of dates.
Release note (bug fix): Previously, attempting to parse
0000-01-01 00:00when involvingtimedid not work aspgdatedoes notunderstand
0000as a year. This PR will fix that behaviour.