Skip to content
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

time: ParseInLocation returns wrong timestamp for DST transition in America/Havana #35508

Closed
jeffreydking opened this issue Nov 11, 2019 · 13 comments
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@jeffreydking
Copy link

jeffreydking commented Nov 11, 2019

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?

loc, _ := time.LoadLocation("America/Havana")
ts, _ := time.ParseInLocation("2006-01-02", "2019-03-10", loc)
fmt.Println(ts)

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:

2019-03-10 01:00:00 -0400 CST

What did you see instead?

2019-03-09 23:00:00 -0500 CST
@andybons andybons changed the title time.ParseInLocation returns wrong timestamp for DST transition in America/Havana time: ParseInLocation returns wrong timestamp for DST transition in America/Havana Nov 11, 2019
@andybons
Copy link
Member

@rsc

@andybons andybons added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. help wanted labels Nov 11, 2019
@andybons andybons added this to the Unplanned milestone Nov 11, 2019
@ianlancetaylor
Copy link
Contributor

As the documentation for time.Date states:

A daylight savings time transition skips or repeats times. For example, in the United States, March 13, 2011 2:15am never occurred, while November 6, 2011 1:15am occurred twice. In such cases, the choice of time zone, and therefore the time, is not well-defined. Date returns a time that is correct in one of the two zones involved in the transition, but it does not guarantee which.

There is no correct answer for midnight on the date of a daylight savings time transition in Cuba, so functions like time.Date and time.ParseInLocation must pick some different time. There is no guarantee as to which different time they will pick.

@jeffreydking
Copy link
Author

jeffreydking commented Nov 18, 2019

@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.

@jeffreydking
Copy link
Author

I respectfully request this issue to be reopened.

@ianlancetaylor
Copy link
Contributor

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

2019-03-09 23:00:00 -0500 CST

I agree that your argument that parsing 2019-03-10 can't return a different date makes sense, but it doesn't account for the complexities of human time. I'm not making an argument about the ambiguity of daylight savings time. I'm saying that your code is asking the time package to display a time that doesn't exist. I know that it doesn't seem like that is what you are doing, but it is. And given that request, there is no right answer.

@ianlancetaylor
Copy link
Contributor

For what it's worth, compare to the GNU date program on GNU/Linux:

> TZ=America/Havana date -d "March 9, 2019"
Sat 09 Mar 2019 12:00:00 AM CST
> TZ=America/Havana date -d "March 10, 2019"
date: invalid date ‘March 10, 2019’
> TZ=America/Havana date -d "March 11, 2019"
Mon 11 Mar 2019 12:00:00 AM CDT

@jeffreydking
Copy link
Author

And given that request, there is no right answer.

I would posit there is a correct answer according to the documentation you cited originally.

A daylight savings time transition skips or repeats times. For example, in the United States, March 13, 2011 2:15am never occurred, while November 6, 2011 1:15am occurred twice. In such cases, the choice of time zone, and therefore the time, is not well-defined. Date returns a time that is correct in one of the two zones involved in the transition, but it does not guarantee which.

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.

@jeffreydking
Copy link
Author

jeffreydking commented Nov 18, 2019

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.

@jeffreydking
Copy link
Author

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!

@ianlancetaylor
Copy link
Contributor

Thanks for your kind words about Go.

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".

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 time.Time value that corresponds to "2019-03-10 00:00:00" in Havana time. But there is no such value. It's not the case that we have a UTC time that we are converting to Havana time. We are being asked for a moment in time, in Havana time, that doesn't exist. There are two possible values to return: the valid time just before "2019-03-10 00:00:00" or the valid time just after "2019-03-10 00:00:00". The time package doesn't specify which one will be returned.

Yes, returning an error for this case would be better. Unfortunately the time.Date function wasn't define to return an error, and we can't change that now. The time.ParseInLocation function can return an error; I don't know how hard it would be to detect this kind of case. I also don't know if we could make that kind of change; it might break existing working code.

@jeffreydking
Copy link
Author

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:

loc, _ := time.LoadLocation("America/Havana")
ts, _ := time.ParseInLocation("2006-01-02", "2019-03-10", loc)
fmt.Println(ts.Unix()) // 1552190400, not 1552194000

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.

@ianlancetaylor
Copy link
Contributor

A consistent way to get to the Unix timestamp for a local day is needed.

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
}

@jeffreydking
Copy link
Author

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.

@golang golang locked and limited conversation to collaborators Nov 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants