Skip to content

time: Time.AddDate(0,0,0) can change underlying instant #50445

Not planned
@berend

Description

@berend

What version of Go are you using (go version)?

$ go version
go version go1.17.2 darwin/amd64

Does this issue reproduce with the latest release?

Yes, this is happening in the playground versions "release" and "go dev branch"

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/berend.kapelle/Library/Caches/go-build"
GOENV="/Users/berend.kapelle/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/berend.kapelle/.go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/berend.kapelle/.go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/opt/go/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/opt/go/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.17.2"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/cm/0bvssb694y1812pzbb3xb7gh0000gn/T/go-build4066904548=/tmp/go-build -gno-record-gcc-switches -fno-common"

(I emptied GONOPROXY, GONOSUMDB and GOPRIVATE, because they are pointing to a private gitlab instance)

What did you do?

  • create time.Date via time.Add() that points to exact DST change
  • .AddDate(0,0,0)

Playground: https://go.dev/play/p/pC1w7JVWqUX

What did you expect to see?

That .AddDate(0,0,0) does not create a different time.Date

What did you see instead?

the time.Date, that .AddDate(0,0,0) created has a difference of one hour

I know, that AddDate does Normalization:

// AddDate normalizes its result in the same way that Date does,
// so, for example, adding one month to October 31 yields
// December 1, the normalized form for November 31.

but I still think, that .AddDate(0,0,0) should create the same time.Date

This behaviour does not appear with .Add(0)

Activity

changed the title [-]time.Date.AddDate(0,0,0) creates a difference when performed on DST change[/-] [+]time: Time.AddDate(0,0,0) can change underlying instant[/+] on Jan 5, 2022
rsc

rsc commented on Jan 5, 2022

@rsc
Contributor

I'm not convinced this is a bug. Right now it seems to be the case that AddDate guarantees that the result matches what you'd get from time.Date given the constituent parts. If it did preserve the old time, then it would have a different inconsistency, which could just as easily be filed as a bug report.

Given the choice between two inconsistent behaviors - you have to pick one! - it seems like we should pick the one we already have instead of changing from one to the other.

tandr

tandr commented on Jan 5, 2022

@tandr

If I may drop my 2 cents (Canadian, so it is like 1.5 US).

Yes, there is an inconsistency possible given the time point chosen. That said - adding a zero assumed to be idempotent operation by default by pretty much every developer. If we are trying to follow a rule of "less surprise", I would argue adding a zero should not change an a result here.

(The fact that it is discovered THAT late makes me think changing (well - fixing) this behavior might not be a big problem).

Thank you.

rsc

rsc commented on Jan 5, 2022

@rsc
Contributor

If we change t.AddDate(0, 0, 0), next someone will point out that
t.AddDate(0, 0, 0) is not the same as t.AddDate(1, 0, 0).AddDate(-1, 0, 0).
(Today it is!)

The fundamental problem is that if you are asking to work in terms of dates,
then 2021 Oct 31 2:00 in Berlin is ambiguous - that happened twice.
It really doesn't make sense to be picky about which one you get once you start working in date space.

zigo101

zigo101 commented on Jan 6, 2022

@zigo101

Who can elaborate more on the background logic? I totally don't get it.

Why does this only happen when N == 1? And only at the date 2021/Oct/31?

package main

import (
	"fmt"
	"time"
)

const N = 1

func main() {
	berlinLocation, err := time.LoadLocation("Europe/Berlin")
	fmt.Println(err) // <nil>
	t1 := time.Date(2021, 10, 31, 1, 0, 0, 0, berlinLocation)
	t2 := t1.Add(time.Hour * N)
 
	t3 := t2.AddDate(0, 0, 0)
	t4 := t2.Add(time.Hour * N)

	fmt.Println(t3.Equal(t2)) // false
	fmt.Println(t3.Equal(t4)) // true
	fmt.Println(t3.Sub(t2))   // 1h0m0s
	fmt.Println(t1) // 2021-10-31 01:00:00 +0200 CEST
	fmt.Println(t2) // 2021-10-31 02:00:00 +0200 CEST
	fmt.Println(t3) // 2021-10-31 02:00:00 +0100 CET
	fmt.Println(t4) // 2021-10-31 02:00:00 +0100 CET
	fmt.Println(t1.UTC()) // 2021-10-30 23:00:00 +0000 UTC
	fmt.Println(t2.UTC()) // 2021-10-31 00:00:00 +0000 UTC
	fmt.Println(t3.UTC()) // 2021-10-31 01:00:00 +0000 UTC
	fmt.Println(t4.UTC()) // 2021-10-31 01:00:00 +0000 UTC
}
ianlancetaylor

ianlancetaylor commented on Jan 6, 2022

@ianlancetaylor
Contributor

@go101 As @rsc said, the time 2021-10-31 02:00:00 in the Europe/Berlin timezone is ambiguous. It happened twice. The first time the clock reached 2021-10-31 02:00:00, it rolled back one hour as the timezone changed from summer time to winter time. One hour later the time 2021-10-31 02:00:00 happened again.

Calculations involving ambiguous times are inevitably surprising. The only question is exactly where the surprises occur.

zigo101

zigo101 commented on Jan 6, 2022

@zigo101

So does it only happen this year (2021), but not other years?

ianlancetaylor

ianlancetaylor commented on Jan 6, 2022

@ianlancetaylor
Contributor

It happens every year that there is a transition from summer time to winter time. Of course, that transition doesn't always happen on October 31. For 2020, for example, use 2020, 10, 25.

zigo101

zigo101 commented on Jan 6, 2022

@zigo101

TIL.
Many thanks.

added
NeedsDecisionFeedback is required from experts, contributors, and/or the community before a change can be made.
on Jan 16, 2022
seankhliao

seankhliao commented on Aug 20, 2022

@seankhliao
Member

Closing as working as intended.
See also #19700

locked and limited conversation to collaborators on Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    FrozenDueToAgeNeedsDecisionFeedback is required from experts, contributors, and/or the community before a change can be made.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @rsc@berend@ianlancetaylor@tandr@gopherbot

        Issue actions

          time: Time.AddDate(0,0,0) can change underlying instant · Issue #50445 · golang/go