From 674f27c026ee87da94bc4499bdfe13a84accd03d Mon Sep 17 00:00:00 2001 From: Mattias Jonsson Date: Fri, 3 Dec 2021 19:08:59 +0100 Subject: [PATCH 1/4] Adjusts UNIX_TIMESTAMP() for non-existing DST values --- expression/builtin_time.go | 2 +- expression/integration_test.go | 4 +++ types/core_time.go | 53 +++++++++++++++++++++++++++ types/core_time_test.go | 65 ++++++++++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 1 deletion(-) diff --git a/expression/builtin_time.go b/expression/builtin_time.go index c6b3cfa669ef9..0c87fe8ec299c 100644 --- a/expression/builtin_time.go +++ b/expression/builtin_time.go @@ -4856,7 +4856,7 @@ func (b *builtinUnixTimestampIntSig) evalIntWithCtx(ctx sessionctx.Context, row } tz := ctx.GetSessionVars().Location() - t, err := val.GoTime(tz) + t, err := val.AdjustedGoTime(tz) if err != nil { return 0, false, nil } diff --git a/expression/integration_test.go b/expression/integration_test.go index 7f01c659c126d..1e80da3b616e0 100644 --- a/expression/integration_test.go +++ b/expression/integration_test.go @@ -2020,6 +2020,10 @@ func (s *testIntegrationSuite2) TestTimeBuiltin(c *C) { result.Check(testkit.Rows("1447410019.012 1447410019.0123")) result = tk.MustQuery("SELECT UNIX_TIMESTAMP('2038-01-19 11:14:07.999999');") result.Check(testkit.Rows("2147483647.999999")) + tk.MustExec("SET time_zone = 'Europe/Vilnius'") + tk.MustQuery("SELECT UNIX_TIMESTAMP('2020-03-29 03:45:00')").Check(testkit.Rows("1585443600")) + tk.MustQuery("SELECT FROM_UNIXTIME(UNIX_TIMESTAMP('2020-03-29 03:45:00'))").Check(testkit.Rows("2020-03-29 04:00:00")) + tk.MustExec("SET time_zone = DEFAULT") result = tk.MustQuery("SELECT TIME_FORMAT('bad string', '%H:%i:%s %p');") result.Check(testkit.Rows("")) diff --git a/types/core_time.go b/types/core_time.go index 161180dd3b166..ba30f009f6bc6 100644 --- a/types/core_time.go +++ b/types/core_time.go @@ -184,6 +184,59 @@ func (t CoreTime) GoTime(loc *gotime.Location) (gotime.Time, error) { return tm, nil } +// FindZoneTransition check for one Time Zone transition within +/- 4h +// Currently the needed functions are not exported, if gotime.Location.lookup would be exported +// then it would be easy to use that directly +func FindZoneTransition(tIn gotime.Time, loc *gotime.Location) (gotime.Time, error) { + // Check most common case first, DST transition on full hour. + // round truncates away from zero! + t2 := tIn.Round(gotime.Hour).Add(-1 * gotime.Hour) + t1 := t2.Round(gotime.Hour).Add(-1 * gotime.Second) + _, offset1 := t1.Zone() + _, offset2 := t2.Zone() + if offset1 != offset2 { + return t2, nil + } + + // Check if any offset change? + t1 = tIn.Add(-4 * gotime.Hour) + t2 = tIn.Add(4 * gotime.Hour) + _, offset1 = t1.Zone() + _, offset2 = t2.Zone() + if offset1 == offset2 { + return tIn, errors.Trace(ErrWrongValue.GenWithStackByArgs(TimeStr, tIn)) + } + + // Check generic case, like for 'Australia/Lord_Howe' + for t2.After(t1.Add(gotime.Second)) { + t := t1.Add(t2.Sub(t1) / 2).Round(gotime.Second) + _, offset := t.Zone() + if offset == offset1 { + t1 = t + } else { + t2 = t + } + } + return t2, nil +} + +// AdjustedGoTime converts Time to GoTime and adjust for invalid DST times +// like during the DST change with increased offset, +// normally moving to Daylight Saving Time. +// see https://github.com/pingcap/tidb/issues/28739 +func (t CoreTime) AdjustedGoTime(loc *gotime.Location) (gotime.Time, error) { + tm, err := t.GoTime(loc) + if err == nil { + return tm, nil + } + + tAdj, err2 := FindZoneTransition(tm, loc) + if err2 == nil { + return tAdj, nil + } + return tm, err +} + // IsLeapYear returns if it's leap year. func (t CoreTime) IsLeapYear() bool { return isLeapYear(t.getYear()) diff --git a/types/core_time_test.go b/types/core_time_test.go index f34e38ab7f8a2..3331cc93799cd 100644 --- a/types/core_time_test.go +++ b/types/core_time_test.go @@ -316,3 +316,68 @@ func TestWeekday(t *testing.T) { require.Equal(t, tt.Expect, weekday.String()) } } + +func TestFindZoneTransition(t *testing.T) { + t.Parallel() + tests := []struct { + TZ string + dt string + Expect string + Success bool + }{ + {"Australia/Lord_Howe", "2020-06-29 03:45:00", "", false}, + {"Australia/Lord_Howe", "2020-10-04 02:15:00", "2020-10-04 02:30:00 +11 +1100", true}, + {"Europe/Vilnius", "2020-03-29 03:45:00", "2020-03-29 04:00:00 EEST +0300", true}, + {"Europe/Vilnius", "2020-10-25 03:45:00", "2020-10-25 03:00:00 EET +0200", true}, + {"Europe/Vilnius", "2020-06-29 03:45:00", "", false}, + {"Europe/Amsterdam", "2020-03-29 02:45:00", "2020-03-29 03:00:00 CEST +0200", true}, + {"Europe/Amsterdam", "2020-10-25 02:35:00", "2020-10-25 02:00:00 CET +0100", true}, + } + + for _, tt := range tests { + loc, err := time.LoadLocation(tt.TZ) + require.NoError(t, err) + tm, err := time.ParseInLocation("2006-01-02 15:04:05", tt.dt, loc) + require.NoError(t, err) + tp, err := FindZoneTransition(tm, loc) + if !tt.Success { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tt.Expect, tp.Format("2006-01-02 15:04:05 MST -0700")) + } + } +} + +func TestAdjustedGoTime(t *testing.T) { + t.Parallel() + tests := []struct { + TZ string + dt CoreTime + Expect string + Success bool + }{ + {"Australia/Lord_Howe", FromDate(2020, 10, 04, 02, 15, 00, 0), "2020-10-04 02:30:00 +11 +1100", true}, + {"Australia/Lord_Howe", FromDate(2020, 06, 29, 03, 45, 00, 0), "2020-06-29 03:45:00 +1030 +1030", true}, + {"Australia/Lord_Howe", FromDate(2020, 04, 04, 01, 45, 00, 0), "2020-04-04 01:45:00 +11 +1100", true}, + {"Europe/Vilnius", FromDate(2020, 03, 29, 03, 45, 00, 0), "2020-03-29 04:00:00 EEST +0300", true}, + {"Europe/Vilnius", FromDate(2020, 10, 25, 03, 45, 00, 0), "2020-10-25 03:45:00 EET +0200", true}, + {"Europe/Vilnius", FromDate(2020, 06, 29, 03, 45, 00, 0), "2020-06-29 03:45:00 EEST +0300", true}, + {"Europe/Amsterdam", FromDate(2020, 03, 29, 02, 45, 00, 0), "2020-03-29 03:00:00 CEST +0200", true}, + {"Europe/Amsterdam", FromDate(2020, 10, 25, 02, 35, 00, 0), "2020-10-25 02:35:00 CET +0100", true}, + {"UTC", FromDate(2020, 2, 31, 02, 35, 00, 0), "", false}, + // TODO: Test out-of-range values? + } + + for _, tt := range tests { + loc, err := time.LoadLocation(tt.TZ) + require.NoError(t, err) + tp, err := tt.dt.AdjustedGoTime(loc) + if !tt.Success { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tt.Expect, tp.Format("2006-01-02 15:04:05 MST -0700")) + } + } +} From d34a3b100acee846433d3ed6d5f29fbd396576c9 Mon Sep 17 00:00:00 2001 From: Mattias Jonsson Date: Fri, 3 Dec 2021 19:24:56 +0100 Subject: [PATCH 2/4] Removed duplicate time.Round() --- types/core_time.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/core_time.go b/types/core_time.go index ba30f009f6bc6..3d47d72d01200 100644 --- a/types/core_time.go +++ b/types/core_time.go @@ -191,7 +191,7 @@ func FindZoneTransition(tIn gotime.Time, loc *gotime.Location) (gotime.Time, err // Check most common case first, DST transition on full hour. // round truncates away from zero! t2 := tIn.Round(gotime.Hour).Add(-1 * gotime.Hour) - t1 := t2.Round(gotime.Hour).Add(-1 * gotime.Second) + t1 := t2.Add(-1 * gotime.Second) _, offset1 := t1.Zone() _, offset2 := t2.Zone() if offset1 != offset2 { From 3e67db8886f629d32f75621601f296b1f8e40e5b Mon Sep 17 00:00:00 2001 From: Mattias Jonsson Date: Tue, 7 Dec 2021 13:04:31 +0100 Subject: [PATCH 3/4] Fixed the same issue in the vectorized implementation --- expression/builtin_time_vec.go | 2 +- expression/integration_test.go | 23 +++++++++++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/expression/builtin_time_vec.go b/expression/builtin_time_vec.go index 90fc1323117f9..13a0135d267b5 100644 --- a/expression/builtin_time_vec.go +++ b/expression/builtin_time_vec.go @@ -2390,7 +2390,7 @@ func (b *builtinUnixTimestampIntSig) vecEvalInt(input *chunk.Chunk, result *chun continue } - t, err := buf.GetTime(i).GoTime(getTimeZone(b.ctx)) + t, err := buf.GetTime(i).AdjustedGoTime(getTimeZone(b.ctx)) if err != nil { i64s[i] = 0 continue diff --git a/expression/integration_test.go b/expression/integration_test.go index 1e80da3b616e0..7c0cb8d228c48 100644 --- a/expression/integration_test.go +++ b/expression/integration_test.go @@ -2020,10 +2020,6 @@ func (s *testIntegrationSuite2) TestTimeBuiltin(c *C) { result.Check(testkit.Rows("1447410019.012 1447410019.0123")) result = tk.MustQuery("SELECT UNIX_TIMESTAMP('2038-01-19 11:14:07.999999');") result.Check(testkit.Rows("2147483647.999999")) - tk.MustExec("SET time_zone = 'Europe/Vilnius'") - tk.MustQuery("SELECT UNIX_TIMESTAMP('2020-03-29 03:45:00')").Check(testkit.Rows("1585443600")) - tk.MustQuery("SELECT FROM_UNIXTIME(UNIX_TIMESTAMP('2020-03-29 03:45:00'))").Check(testkit.Rows("2020-03-29 04:00:00")) - tk.MustExec("SET time_zone = DEFAULT") result = tk.MustQuery("SELECT TIME_FORMAT('bad string', '%H:%i:%s %p');") result.Check(testkit.Rows("")) @@ -10673,3 +10669,22 @@ func (s *testIntegrationSuite) TestIssue30101(c *C) { tk.MustExec("insert into t1 values(9223372036854775808, 9223372036854775809);") tk.MustQuery("select greatest(c1, c2) from t1;").Sort().Check(testkit.Rows("9223372036854775809")) } + +func (s *testIntegrationSuite) TestIssue28739(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec(`USE test`) + tk.MustExec("SET time_zone = 'Europe/Vilnius'") + tk.MustQuery("SELECT UNIX_TIMESTAMP('2020-03-29 03:45:00')").Check(testkit.Rows("1585443600")) + tk.MustQuery("SELECT FROM_UNIXTIME(UNIX_TIMESTAMP('2020-03-29 03:45:00'))").Check(testkit.Rows("2020-03-29 04:00:00")) + tk.MustExec(`DROP TABLE IF EXISTS t`) + tk.MustExec(`CREATE TABLE t (dt DATETIME NULL)`) + defer tk.MustExec(`DROP TABLE t`) + // Test the vector implememtation + tk.MustExec(`INSERT INTO t VALUES ('2021-10-31 02:30:00'), ('2021-03-28 02:30:00'), ('2020-10-04 02:15:00'), ('2020-03-29 03:45:00'), (NULL)`) + tk.MustQuery(`SELECT dt, UNIX_TIMESTAMP(dt) FROM t`).Sort().Check(testkit.Rows( + "2020-03-29 03:45:00 1585443600", + "2020-10-04 02:15:00 1601766900", + "2021-03-28 02:30:00 1616891400", + "2021-10-31 02:30:00 1635636600", + " ")) +} From bacc9b5ce36f2e65cb5b867e72748d2be4e548f8 Mon Sep 17 00:00:00 2001 From: Mattias Jonsson Date: Wed, 8 Dec 2021 11:06:02 +0100 Subject: [PATCH 4/4] removed unused argument and added fractional seconds tests --- types/core_time.go | 4 ++-- types/core_time_test.go | 21 +++++++++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/types/core_time.go b/types/core_time.go index 3d47d72d01200..64820f98aaf79 100644 --- a/types/core_time.go +++ b/types/core_time.go @@ -187,7 +187,7 @@ func (t CoreTime) GoTime(loc *gotime.Location) (gotime.Time, error) { // FindZoneTransition check for one Time Zone transition within +/- 4h // Currently the needed functions are not exported, if gotime.Location.lookup would be exported // then it would be easy to use that directly -func FindZoneTransition(tIn gotime.Time, loc *gotime.Location) (gotime.Time, error) { +func FindZoneTransition(tIn gotime.Time) (gotime.Time, error) { // Check most common case first, DST transition on full hour. // round truncates away from zero! t2 := tIn.Round(gotime.Hour).Add(-1 * gotime.Hour) @@ -230,7 +230,7 @@ func (t CoreTime) AdjustedGoTime(loc *gotime.Location) (gotime.Time, error) { return tm, nil } - tAdj, err2 := FindZoneTransition(tm, loc) + tAdj, err2 := FindZoneTransition(tm) if err2 == nil { return tAdj, nil } diff --git a/types/core_time_test.go b/types/core_time_test.go index 3331cc93799cd..07e4d040a5c2c 100644 --- a/types/core_time_test.go +++ b/types/core_time_test.go @@ -327,11 +327,19 @@ func TestFindZoneTransition(t *testing.T) { }{ {"Australia/Lord_Howe", "2020-06-29 03:45:00", "", false}, {"Australia/Lord_Howe", "2020-10-04 02:15:00", "2020-10-04 02:30:00 +11 +1100", true}, + {"Australia/Lord_Howe", "2020-10-04 02:29:59", "2020-10-04 02:30:00 +11 +1100", true}, + {"Australia/Lord_Howe", "2020-10-04 02:29:59.99", "2020-10-04 02:30:00 +11 +1100", true}, + {"Australia/Lord_Howe", "2020-10-04 02:30:00.0001", "2020-10-04 02:30:00 +11 +1100", true}, + {"Australia/Lord_Howe", "2020-10-04 02:30:00", "2020-10-04 02:30:00 +11 +1100", true}, + {"Australia/Lord_Howe", "2020-10-04 02:30:01", "2020-10-04 02:30:00 +11 +1100", true}, {"Europe/Vilnius", "2020-03-29 03:45:00", "2020-03-29 04:00:00 EEST +0300", true}, {"Europe/Vilnius", "2020-10-25 03:45:00", "2020-10-25 03:00:00 EET +0200", true}, {"Europe/Vilnius", "2020-06-29 03:45:00", "", false}, {"Europe/Amsterdam", "2020-03-29 02:45:00", "2020-03-29 03:00:00 CEST +0200", true}, {"Europe/Amsterdam", "2020-10-25 02:35:00", "2020-10-25 02:00:00 CET +0100", true}, + {"Europe/Amsterdam", "2020-03-29 02:59:59", "2020-03-29 03:00:00 CEST +0200", true}, + {"Europe/Amsterdam", "2020-03-29 02:59:59.999999999", "2020-03-29 03:00:00 CEST +0200", true}, + {"Europe/Amsterdam", "2020-03-29 03:00:00.000000001", "2020-03-29 03:00:00 CEST +0200", true}, } for _, tt := range tests { @@ -339,12 +347,12 @@ func TestFindZoneTransition(t *testing.T) { require.NoError(t, err) tm, err := time.ParseInLocation("2006-01-02 15:04:05", tt.dt, loc) require.NoError(t, err) - tp, err := FindZoneTransition(tm, loc) + tp, err := FindZoneTransition(tm) if !tt.Success { require.Error(t, err) } else { require.NoError(t, err) - require.Equal(t, tt.Expect, tp.Format("2006-01-02 15:04:05 MST -0700")) + require.Equal(t, tt.Expect, tp.Format("2006-01-02 15:04:05.999999999 MST -0700")) } } } @@ -357,16 +365,21 @@ func TestAdjustedGoTime(t *testing.T) { Expect string Success bool }{ + {"Australia/Lord_Howe", FromDate(2020, 10, 04, 01, 59, 59, 997), "2020-10-04 01:59:59.000997 +1030 +1030", true}, + {"Australia/Lord_Howe", FromDate(2020, 10, 04, 02, 00, 00, 0), "2020-10-04 02:30:00 +11 +1100", true}, {"Australia/Lord_Howe", FromDate(2020, 10, 04, 02, 15, 00, 0), "2020-10-04 02:30:00 +11 +1100", true}, + {"Australia/Lord_Howe", FromDate(2020, 10, 04, 02, 29, 59, 999999), "2020-10-04 02:30:00 +11 +1100", true}, + {"Australia/Lord_Howe", FromDate(2020, 10, 04, 02, 30, 00, 1), "2020-10-04 02:30:00.000001 +11 +1100", true}, {"Australia/Lord_Howe", FromDate(2020, 06, 29, 03, 45, 00, 0), "2020-06-29 03:45:00 +1030 +1030", true}, {"Australia/Lord_Howe", FromDate(2020, 04, 04, 01, 45, 00, 0), "2020-04-04 01:45:00 +11 +1100", true}, {"Europe/Vilnius", FromDate(2020, 03, 29, 03, 45, 00, 0), "2020-03-29 04:00:00 EEST +0300", true}, + {"Europe/Vilnius", FromDate(2020, 03, 29, 03, 59, 59, 456789), "2020-03-29 04:00:00 EEST +0300", true}, + {"Europe/Vilnius", FromDate(2020, 03, 29, 04, 00, 01, 130000), "2020-03-29 04:00:01.13 EEST +0300", true}, {"Europe/Vilnius", FromDate(2020, 10, 25, 03, 45, 00, 0), "2020-10-25 03:45:00 EET +0200", true}, {"Europe/Vilnius", FromDate(2020, 06, 29, 03, 45, 00, 0), "2020-06-29 03:45:00 EEST +0300", true}, {"Europe/Amsterdam", FromDate(2020, 03, 29, 02, 45, 00, 0), "2020-03-29 03:00:00 CEST +0200", true}, {"Europe/Amsterdam", FromDate(2020, 10, 25, 02, 35, 00, 0), "2020-10-25 02:35:00 CET +0100", true}, {"UTC", FromDate(2020, 2, 31, 02, 35, 00, 0), "", false}, - // TODO: Test out-of-range values? } for _, tt := range tests { @@ -377,7 +390,7 @@ func TestAdjustedGoTime(t *testing.T) { require.Error(t, err) } else { require.NoError(t, err) - require.Equal(t, tt.Expect, tp.Format("2006-01-02 15:04:05 MST -0700")) + require.Equal(t, tt.Expect, tp.Format("2006-01-02 15:04:05.999999999 MST -0700")) } } }