encode: don't shift local datetimes/dates/times to UTC#488
Conversation
|
They're not set to UTC, they're set to I'm not sure if setting it to the computer's local timezone is necessarily better. We should maybe export these though. And document it better. I agree all of this is confusing and annoying (it also came up in the pq SQL driver recently). |
|
The main issue here by the way is being able to distinguish between: If we'd just parse it to time.Local, that would get lost. And e.g. I'm not saying we can't improve something here, maybe, but I'm not sure this PR as it is now is really an improvement. Open to suggestions, but round-tripping the format is more useful IMO. |
|
Oops, didn't mean to close. |
|
Right — and Iʼm not changing any of that. Decoding is untouched, so values still come back tagged with The only change is the format call itself. Those sentinels are So this isnʼt "parse to |
|
Okay, thanks – maybe I misunderstood the diff then. I'll have a closer look later. |
9dde78f to
dff0d5a
Compare
A local datetime, date, or time carries no timezone offset, so its value is the literal wall-clock time. The decoder stores it as such using time.ParseInLocation, but the encoder formatted it with v.In(time.UTC) first. On any machine not in UTC this shifted the value by the local offset, so Decode -> Encode -> Decode changed the time (e.g. a local time of 07:32:00 became 11:32:00 in UTC-4, and a near-midnight local datetime could even roll over to a different day). Format the wall-clock value directly instead. Also fix the toml-test helper's parseTime to build local times with time.ParseInLocation so it matches how the decoder constructs them.
dff0d5a to
24ca3d8
Compare
|
Thanks! |
I noticed that local datetimes, dates, and times don't survive a round trip on any machine that isn't in UTC: decoding then re-encoding shifts the value by the local timezone offset.
A local time of
07:32:00comes back as11:32:00, and a near-midnight local datetime can even roll over to a different day. These types have no offset, so their value is the literal wall-clock time — the decoder already stores it that way viatime.ParseInLocation, but the encoder ranv.In(time.UTC)before formatting, which moved the wall clock.The fix formats the wall-clock value directly. I also updated the
toml-testhelper'sparseTimeto build local times withtime.ParseInLocationso it constructs them the same way the decoder does (the previoustime.Parse(...).In(l)and the encoder's UTC shift were cancelling each other out, which is why the conformance suite didn't catch this).Tests pass under several timezones (
UTC,America/New_York,Pacific/Kiritimati); added a round-trip test for the local types. The existingossfuzzround-trip harness didn't surface this because it only re-decodes the output, it doesn't compare values.