-
Notifications
You must be signed in to change notification settings - Fork 18k
time: ParseInLocation returns wrong timestamp for DST transition in America/Havana #35508
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
Comments
As the documentation for
There is no correct answer for midnight on the date of a daylight savings time transition in Cuba, so functions like |
@ianlancetaylor Respectfully, you miss the critical point. The issue is not an ambiguous implementation of DST. The salient issue is simply that the timestamp for "2019-03-10" must occur on March 10th, not March 9th. No implementation of DST can force a day to occur on a previous day. |
I respectfully request this issue to be reopened. |
The time 2019-03-10 00:00 does not exist in Cuba. When asked for a time that does not exist, we must return either an earlier time that exists or a later time that exists. We document that we make no guarantees as to whether we return the earlier or later time. In this case we return the earlier time. Your program is essentially equivalent to package main
import (
"fmt"
"log"
"time"
)
func main() {
loc, err := time.LoadLocation("America/Havana")
if err != nil {
log.Fatal(err)
}
fmt.Println(time.Date(2019, 3, 10, 0, 0, 0, 0, loc))
} which prints
I agree that your argument that parsing |
For what it's worth, compare to the GNU date program on GNU/Linux:
|
I would posit there is a correct answer according to the documentation you cited originally.
This is a "spring forward" event. There is no repeating of local time. So there is no ambiguity here. Every local second remains unique during the "spring forward" event. To demonstrate this: One second before the requested time would be "2019-03-09 23:59:59 -0500 CST". Converting this to UTC, we get a corresponding time of "2019-03-10 04:59:59 +0000 UTC". The following second (UTC) time is "2019-03-10 05:00:00 +0000 UTC". When converted back to Havana time, this can only convert to "2019-03-10 01:00:00 -0400 CST". Another way of looking at this. Suppose there is ambiguity here. The time will either be returned with a -0400 or a -0500 UTC offset. If the function returns a -0400 offset, the valid corresponding time would be "2019-03-10 00:00:00 -0400 CST". This is the time with a -0400 UTC offset that corresponds to the correct Unix Timestamp. (The returned value of "2019-03-09 23:00:00 -0500 CST" corresponds to a Unix Timestamp that is off by 3600 seconds.) However, we know that no local midnight occurred, so this is not be the correct response. But it is still preferred to the actual response, since it (1) has the correct Unix Timestamp, and (2) has a correct local date portion of the timestamp. Alternatively, if a UTC offset of -0500 is returned, the result would be "2019-03-10 01:00:00 -0400 CST". This is obviously the preferred answer, as it is the only time that existed locally. It also maps the the correct Unix Timestamp. A third way of looking at this is to consider the "fall back" event in the autumn. There are 2 timestamps that may be returned (because of the ambiguity). Both of which will convert to the same UTC time. (They would be off 1 hour in the local time, but this would be offset by 1 hour in the UTC offset portion of the timestamp). During the spring, the actual response maps to a different times entirely. If the correct options for the original query are "2019-03-09 23:00:00 -0500 CST" (the actual response) and "2019-03-10 01:00:00 -0400 CST" (the response I propose as correct), these map to 2 entirely different UTC times. They do not correspond. They are not equivalent. Only one is correct. It is not the latter. |
I also understand your point about feeding an invalid time into the method. If that is how you see this, and you don't plan on handling it as it would intuitively be handled, an "invalid date" error (similar to the GNU date program on GNU/Linux) would be preferred to the wrong date/time being currently returned. |
As an aside @ianlancetaylor, thank you for all the hard work you have done with Golang. This is an amazing language, and a pleasure to code with daily. You have made my life much better! |
Thanks for your kind words about Go.
I don't think this is the right description of what is happening here. It's not the case that we have a UTC time that we are converting to Havana time. The code is asking for the Yes, returning an error for this case would be better. Unfortunately the |
This will be my last attempt to persuade you, then I'll let it rest. Thank you for bearing with me. This is not just a theoretical edge case for me. It breaks things - as I'll explain briefly below. I hear your point, and concede this is not a bug as defined. So please consider this a feature request. A small modification will bring the time package into end-user expected behavior. We have a program that returns weather data for day periods for any provided lat/lng. The data is databased by Unix Timestamp (which is not DST sensitive) with the typical convention that a period of time is represented by the first valid second of that period. There is a valid Unix timestamp for 2019-03-10 in Havana, it is 1552194000. Using the time package:
There is no data in the database for 1552190400, since this is the wrong Unix Timestamp (off by 3600 seconds). A consistent way to get to the Unix timestamp for a local day is needed. Since the time package's DST behavior is explicitly undefined, it is not a breaking change to alter the behavior. The proposed change is: when a time is undefined, round forward (never backward) 1 hour. This will provide consistent and expected translation to the UTC/Unix timestamp. Since the "fall back" event does not cause undefined time (rather non-unique time), this change would not have an affect on that circumstance. Thank you for considering this request. |
Well, now that you are aware of the problem, you could use func firstTimeInDay(year int, month time.Month, day int, loc *time.Location) time.Time {
t := time.Date(year, month, day, 0, 0, 0, 0, loc)
for t.Month() != month || t.Day() != day {
t = t.Add(time.Hour)
}
return t
} |
Yes. This is my current workaround. But it is a bit of a hack, and only available to people who are aware of the issue (which only surfaced for us because of some extensive testing we regularly run). Thanks again, and as I promised, I'll let this lie now. |
What version of Go are you using (
go version
)?go1.12.6
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?darwin/amd64
What did you do?
What did you expect to see?
This is an edge case where Cuba has declared DST be applied at 12 midnight (not 1am like in the USA). So, there is no midnight for 2019-03-10 in the America/Havana timezone. As such, the timestamp for the day should be the first available second of that day:
What did you see instead?
The text was updated successfully, but these errors were encountered: