From 24d970fc46070d278bc9495199880839d35e07b7 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Sun, 19 Dec 2021 17:57:45 -0600 Subject: [PATCH] executor: improve SET sysvar=DEFAULT handling (#29680) close pingcap/tidb#29670 --- executor/set_test.go | 28 +++++++++++++++++++++++++++- expression/helper_test.go | 2 +- sessionctx/variable/sysvar.go | 6 ------ sessionctx/variable/variable.go | 5 ----- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/executor/set_test.go b/executor/set_test.go index 6b166059e6921..da121e77b2422 100644 --- a/executor/set_test.go +++ b/executor/set_test.go @@ -111,7 +111,7 @@ func (s *testSerialSuite1) TestSetVar(c *C) { tk.MustQuery(`select @@global.low_priority_updates;`).Check(testkit.Rows("0")) tk.MustExec(`set @@global.low_priority_updates="ON";`) tk.MustQuery(`select @@global.low_priority_updates;`).Check(testkit.Rows("1")) - tk.MustExec(`set @@global.low_priority_updates=DEFAULT;`) // It will be set to compiled-in default value. + tk.MustExec(`set @@global.low_priority_updates=DEFAULT;`) // It will be set to default var value. tk.MustQuery(`select @@global.low_priority_updates;`).Check(testkit.Rows("0")) // For session tk.MustQuery(`select @@session.low_priority_updates;`).Check(testkit.Rows("0")) @@ -1387,6 +1387,32 @@ func (s *testSuite5) TestEnableNoopFunctionsVar(c *C) { } +// https://github.com/pingcap/tidb/issues/29670 +func (s *testSuite5) TestDefaultBehavior(c *C) { + tk := testkit.NewTestKit(c, s.store) + + tk.MustQuery("SELECT @@default_storage_engine").Check(testkit.Rows("InnoDB")) + tk.MustExec("SET GLOBAL default_storage_engine = 'somethingweird'") + tk.MustExec("SET default_storage_engine = 'MyISAM'") + tk.MustQuery("SELECT @@default_storage_engine").Check(testkit.Rows("MyISAM")) + tk.MustExec("SET default_storage_engine = DEFAULT") // reads from global value + tk.MustQuery("SELECT @@default_storage_engine").Check(testkit.Rows("somethingweird")) + tk.MustExec("SET @@SESSION.default_storage_engine = @@GLOBAL.default_storage_engine") // example from MySQL manual + tk.MustQuery("SELECT @@default_storage_engine").Check(testkit.Rows("somethingweird")) + tk.MustExec("SET GLOBAL default_storage_engine = 'somethingweird2'") + tk.MustExec("SET default_storage_engine = @@GLOBAL.default_storage_engine") // variation of example + tk.MustQuery("SELECT @@default_storage_engine").Check(testkit.Rows("somethingweird2")) + tk.MustExec("SET default_storage_engine = DEFAULT") // restore default again for session global + tk.MustExec("SET GLOBAL default_storage_engine = DEFAULT") // restore default for global + tk.MustQuery("SELECT @@SESSION.default_storage_engine, @@GLOBAL.default_storage_engine").Check(testkit.Rows("somethingweird2 InnoDB")) + + // Try sql_mode option which has validation + err := tk.ExecToErr("SET GLOBAL sql_mode = 'DEFAULT'") // illegal now + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, `ERROR 1231 (42000): Variable 'sql_mode' can't be set to the value of 'DEFAULT'`) + tk.MustExec("SET GLOBAL sql_mode = DEFAULT") +} + func (s *testSuite5) TestRemovedSysVars(c *C) { tk := testkit.NewTestKit(c, s.store) diff --git a/expression/helper_test.go b/expression/helper_test.go index b7e00c221e141..63a9ca4137ed4 100644 --- a/expression/helper_test.go +++ b/expression/helper_test.go @@ -42,7 +42,7 @@ func TestGetTimeValue(t *testing.T) { require.Equal(t, "2012-12-12 00:00:00", timeValue.String()) sessionVars := ctx.GetSessionVars() - err = variable.SetSessionSystemVar(sessionVars, "timestamp", "default") + err = variable.SetSessionSystemVar(sessionVars, "timestamp", "0") require.NoError(t, err) v, err = GetTimeValue(ctx, "2012-12-12 00:00:00", mysql.TypeTimestamp, types.MinFsp) require.NoError(t, err) diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index fc7ce09cae6a7..3491f28bc73dc 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -120,12 +120,6 @@ var defaultSysVars = []*SysVar{ } timestamp := s.StmtCtx.GetOrStoreStmtCache(stmtctx.StmtNowTsCacheKey, time.Now()).(time.Time) return types.ToString(float64(timestamp.UnixNano()) / float64(time.Second)) - }, GetGlobal: func(s *SessionVars) (string, error) { - // The Timestamp sysvar will have GetGlobal func even though it does not have global scope. - // It's GetGlobal func will only be called when "set timestamp = default". - // Setting timestamp to DEFAULT causes its value to be the current date and time as of the time it is accessed. - // See https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_timestamp - return DefTimestamp, nil }}, {Scope: ScopeGlobal | ScopeSession, Name: CollationDatabase, Value: mysql.DefaultCollationName, skipInit: true, Validation: func(vars *SessionVars, normalizedValue string, originalValue string, scope ScopeFlag) (string, error) { return checkCollation(vars, normalizedValue, originalValue, scope) diff --git a/sessionctx/variable/variable.go b/sessionctx/variable/variable.go index 675ca3bdc0887..aeb65a8257d9f 100644 --- a/sessionctx/variable/variable.go +++ b/sessionctx/variable/variable.go @@ -261,11 +261,6 @@ func (sv *SysVar) Validate(vars *SessionVars, value string, scope ScopeFlag) (st // ValidateFromType provides automatic validation based on the SysVar's type func (sv *SysVar) ValidateFromType(vars *SessionVars, value string, scope ScopeFlag) (string, error) { - // The string "DEFAULT" is a special keyword in MySQL, which restores - // the compiled sysvar value. In which case we can skip further validation. - if strings.EqualFold(value, "DEFAULT") { - return sv.Value, nil - } // Some sysvars in TiDB have a special behavior where the empty string means // "use the config file value". This needs to be cleaned up once the behavior // for instance variables is determined.