From 13af0e0f0cda2244ce25e93cb3f6a4b8097aaa82 Mon Sep 17 00:00:00 2001 From: Yiding Cui Date: Fri, 8 Sep 2023 16:23:18 +0800 Subject: [PATCH 1/2] sessionctx: add testcase for set_var hint --- sessionctx/variable/session.go | 2 +- sessionctx/variable/setvar_test.go | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index b903090472b76..d0458a64ee7a0 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -2536,7 +2536,7 @@ func (s *SessionVars) SetSystemVarWithOldValAsRet(name string, val string) (stri if err != nil { return "", err } - oldV := sv.Value + oldV := s.systems[sv.Name] return oldV, sv.SetSessionFromHook(s, val) } diff --git a/sessionctx/variable/setvar_test.go b/sessionctx/variable/setvar_test.go index eb8eaa11e15b2..5446ddedbb6e8 100644 --- a/sessionctx/variable/setvar_test.go +++ b/sessionctx/variable/setvar_test.go @@ -152,3 +152,15 @@ func TestSetVarHintBreakCache(t *testing.T) { tk.MustExec("SELECT * FROM t WHERE b < 5 AND a = 2;") tk.MustQuery("select @@last_plan_from_cache;").Check(testkit.Rows("1")) } + +func TestSetVarHintWithCurrentValNotDefaultVal(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + + tk.MustQuery("select @@max_execution_time").Check(testkit.Rows("0")) + tk.MustExec("set @@max_execution_time=1000") + tk.MustQuery("select @@max_execution_time").Check(testkit.Rows("1000")) + tk.MustQuery("select /*+ set_var(max_execution_time=100) */ @@max_execution_time").Check(testkit.Rows("100")) + // The value is the changed value 1000, not the default value 0. + tk.MustQuery("select @@max_execution_time").Check(testkit.Rows("1000")) +} From 9386dbf09f8bd2cce5d500bd4f30e97cef404ce7 Mon Sep 17 00:00:00 2001 From: Yiding Cui Date: Fri, 8 Sep 2023 18:24:29 +0800 Subject: [PATCH 2/2] change the value to get the old val and modify tests --- sessionctx/variable/session.go | 7 ++++++- sessionctx/variable/setvar_test.go | 32 +++++++++++++++++++----------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index d0458a64ee7a0..9b8eeae24ca0e 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -2536,7 +2536,12 @@ func (s *SessionVars) SetSystemVarWithOldValAsRet(name string, val string) (stri if err != nil { return "", err } - oldV := s.systems[sv.Name] + // The map s.systems[sv.Name] is lazy initialized. If we directly read it, we might read empty result. + // Since this code path is not a hot path, we directly call GetSessionOrGlobalSystemVar to get the value safely. + oldV, err := s.GetSessionOrGlobalSystemVar(context.Background(), sv.Name) + if err != nil { + return "", err + } return oldV, sv.SetSessionFromHook(s, val) } diff --git a/sessionctx/variable/setvar_test.go b/sessionctx/variable/setvar_test.go index 5446ddedbb6e8..4e5db8b274ff9 100644 --- a/sessionctx/variable/setvar_test.go +++ b/sessionctx/variable/setvar_test.go @@ -110,6 +110,26 @@ func TestSetVarNonStringOrEnum(t *testing.T) { tk.MustQuery(fmt.Sprintf("select @@%v", c.varName)).Check(r) } } + + tk.MustQuery("select @@max_execution_time").Check(testkit.Rows("0")) + tk.MustExec("set @@max_execution_time=1000") + tk.MustQuery("select @@max_execution_time").Check(testkit.Rows("1000")) + tk.MustQuery("select /*+ set_var(max_execution_time=100) */ @@max_execution_time").Check(testkit.Rows("100")) + // The value is the changed value 1000, not the default value 0. + tk.MustQuery("select @@max_execution_time").Check(testkit.Rows("1000")) + + tk.MustExec("set @@global.max_execution_time=1000") + + tk2 := testkit.NewTestKit(t, store) + tk2.MustQuery("select @@max_execution_time").Check(testkit.Rows("1000")) + tk2.MustQuery("select /*+ set_var(max_execution_time=100) */ @@max_execution_time").Check(testkit.Rows("100")) + // The value is the global value 1000, not the default value 0. + tk2.MustQuery("select @@max_execution_time").Check(testkit.Rows("1000")) + tk2.MustExec("set @@max_execution_time=2000") + tk2.MustQuery("select @@max_execution_time").Check(testkit.Rows("2000")) + tk2.MustQuery("select /*+ set_var(max_execution_time=100) */ @@max_execution_time").Check(testkit.Rows("100")) + // The value is the changed value 2000, not the default value 0 or the global value 1000. + tk2.MustQuery("select @@max_execution_time").Check(testkit.Rows("2000")) } func TestSetVarStringOrEnum(t *testing.T) { @@ -152,15 +172,3 @@ func TestSetVarHintBreakCache(t *testing.T) { tk.MustExec("SELECT * FROM t WHERE b < 5 AND a = 2;") tk.MustQuery("select @@last_plan_from_cache;").Check(testkit.Rows("1")) } - -func TestSetVarHintWithCurrentValNotDefaultVal(t *testing.T) { - store := testkit.CreateMockStore(t) - tk := testkit.NewTestKit(t, store) - - tk.MustQuery("select @@max_execution_time").Check(testkit.Rows("0")) - tk.MustExec("set @@max_execution_time=1000") - tk.MustQuery("select @@max_execution_time").Check(testkit.Rows("1000")) - tk.MustQuery("select /*+ set_var(max_execution_time=100) */ @@max_execution_time").Check(testkit.Rows("100")) - // The value is the changed value 1000, not the default value 0. - tk.MustQuery("select @@max_execution_time").Check(testkit.Rows("1000")) -}