executor: improve SET sysvar=DEFAULT handling (#29680)

close #29670

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)
 

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)

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)

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.