From d65ccb3ee12b1975e9b55a738c38d56976af4b71 Mon Sep 17 00:00:00 2001 From: "vitess-bot[bot]" <108069721+vitess-bot[bot]@users.noreply.github.com> Date: Thu, 16 Jan 2025 07:15:34 +0100 Subject: [PATCH 1/2] Cherry-pick 5468f5db7db117f955aaf9c56182f05fc496c14b Signed-off-by: Dirkjan Bussink --- go/vt/vtgate/engine/fake_vcursor_test.go | 2 +- go/vt/vtgate/evalengine/fn_time.go | 4 ++-- go/vt/vtgate/safe_session.go | 17 +++++++++++++---- go/vt/vtgate/safe_session_test.go | 18 ++++++++++++------ 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/go/vt/vtgate/engine/fake_vcursor_test.go b/go/vt/vtgate/engine/fake_vcursor_test.go index 08a40c0d835..330cd725920 100644 --- a/go/vt/vtgate/engine/fake_vcursor_test.go +++ b/go/vt/vtgate/engine/fake_vcursor_test.go @@ -140,7 +140,7 @@ func (t *noopVCursor) Environment() *vtenv.Environment { } func (t *noopVCursor) TimeZone() *time.Location { - return nil + return time.Local } func (t *noopVCursor) SQLMode() string { diff --git a/go/vt/vtgate/evalengine/fn_time.go b/go/vt/vtgate/evalengine/fn_time.go index 8245de7669f..f01d7c7f9bb 100644 --- a/go/vt/vtgate/evalengine/fn_time.go +++ b/go/vt/vtgate/evalengine/fn_time.go @@ -232,7 +232,7 @@ func (call *builtinNow) constant() bool { func (call *builtinSysdate) eval(env *ExpressionEnv) (eval, error) { now := SystemTime() - if tz := env.currentTimezone(); tz != nil { + if tz := env.currentTimezone(); tz != time.Local { now = now.In(tz) } return newEvalDateTime(datetime.NewDateTimeFromStd(now), int(call.prec), false), nil @@ -673,7 +673,7 @@ func (b *builtinFromUnixtime) eval(env *ExpressionEnv) (eval, error) { } t := time.Unix(sec, frac) - if tz := env.currentTimezone(); tz != nil { + if tz := env.currentTimezone(); tz != time.Local { t = t.In(tz) } diff --git a/go/vt/vtgate/safe_session.go b/go/vt/vtgate/safe_session.go index 45fff46f629..9828d0a77d4 100644 --- a/go/vt/vtgate/safe_session.go +++ b/go/vt/vtgate/safe_session.go @@ -26,7 +26,7 @@ import ( "google.golang.org/protobuf/proto" "vitess.io/vitess/go/mysql/datetime" - + "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/srvtopo" "vitess.io/vitess/go/vt/sysvars" @@ -563,13 +563,22 @@ func (session *SafeSession) HasSystemVariables() (found bool) { func (session *SafeSession) TimeZone() *time.Location { session.mu.Lock() - tz, ok := session.SystemVariables["time_zone"] + zoneSQL, ok := session.SystemVariables["time_zone"] session.mu.Unlock() if !ok { - return nil + return time.Local + } + + tz, err := sqltypes.DecodeStringSQL(zoneSQL) + if err != nil { + return time.Local + } + + loc, err := datetime.ParseTimeZone(tz) + if err != nil { + return time.Local } - loc, _ := datetime.ParseTimeZone(tz) return loc } diff --git a/go/vt/vtgate/safe_session_test.go b/go/vt/vtgate/safe_session_test.go index 21bb2d6697a..f3a78d7f497 100644 --- a/go/vt/vtgate/safe_session_test.go +++ b/go/vt/vtgate/safe_session_test.go @@ -73,25 +73,31 @@ func TestTimeZone(t *testing.T) { want string }{ { - tz: "Europe/Amsterdam", + tz: "", + want: time.Local.String(), + }, + { + tz: "'Europe/Amsterdam'", want: "Europe/Amsterdam", }, { - tz: "+02:00", + tz: "'+02:00'", want: "UTC+02:00", }, { tz: "foo", - want: (*time.Location)(nil).String(), + want: time.Local.String(), }, } for _, tc := range testCases { t.Run(tc.tz, func(t *testing.T) { + sysvars := map[string]string{} + if tc.tz != "" { + sysvars["time_zone"] = tc.tz + } session := NewSafeSession(&vtgatepb.Session{ - SystemVariables: map[string]string{ - "time_zone": tc.tz, - }, + SystemVariables: sysvars, }) assert.Equal(t, tc.want, session.TimeZone().String()) From 1d32f2f80d45b9c60a3e6e2f6d3c73f96134925c Mon Sep 17 00:00:00 2001 From: Dirkjan Bussink Date: Mon, 20 Jan 2025 11:28:44 +0100 Subject: [PATCH 2/2] Fix only panic, leave other bug for now Signed-off-by: Dirkjan Bussink --- go/vt/vtgate/safe_session.go | 8 +------- go/vt/vtgate/safe_session_test.go | 4 ++-- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/go/vt/vtgate/safe_session.go b/go/vt/vtgate/safe_session.go index 9828d0a77d4..54aa3f752de 100644 --- a/go/vt/vtgate/safe_session.go +++ b/go/vt/vtgate/safe_session.go @@ -26,7 +26,6 @@ import ( "google.golang.org/protobuf/proto" "vitess.io/vitess/go/mysql/datetime" - "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/srvtopo" "vitess.io/vitess/go/vt/sysvars" @@ -563,18 +562,13 @@ func (session *SafeSession) HasSystemVariables() (found bool) { func (session *SafeSession) TimeZone() *time.Location { session.mu.Lock() - zoneSQL, ok := session.SystemVariables["time_zone"] + tz, ok := session.SystemVariables["time_zone"] session.mu.Unlock() if !ok { return time.Local } - tz, err := sqltypes.DecodeStringSQL(zoneSQL) - if err != nil { - return time.Local - } - loc, err := datetime.ParseTimeZone(tz) if err != nil { return time.Local diff --git a/go/vt/vtgate/safe_session_test.go b/go/vt/vtgate/safe_session_test.go index f3a78d7f497..199b9239bf2 100644 --- a/go/vt/vtgate/safe_session_test.go +++ b/go/vt/vtgate/safe_session_test.go @@ -77,11 +77,11 @@ func TestTimeZone(t *testing.T) { want: time.Local.String(), }, { - tz: "'Europe/Amsterdam'", + tz: "Europe/Amsterdam", want: "Europe/Amsterdam", }, { - tz: "'+02:00'", + tz: "+02:00", want: "UTC+02:00", }, {