Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

executor: improve SET sysvar=DEFAULT handling #29680

Merged
merged 9 commits into from
Dec 19, 2021
28 changes: 27 additions & 1 deletion executor/set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion expression/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 0 additions & 6 deletions sessionctx/variable/sysvar.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 0 additions & 5 deletions sessionctx/variable/variable.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down