implement correct fix

pingcap · Dec 16, 2021 · 3aaeabc · 3aaeabc
1 parent 5d05315
commit 3aaeabc
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 14 deletions.
diff --git a/executor/set.go b/executor/set.go
@@ -115,11 +115,11 @@ func (e *SetExecutor) setSysVariable(ctx context.Context, name string, v *expres
 		}
 		return variable.ErrUnknownSystemVar.GenWithStackByArgs(name)
 	}
-	valStr, err := e.extractVarValue(v, sysVar)
-	if err != nil {
-		return err
-	}
 	if v.IsGlobal {
+		valStr, err := e.getVarValue(v, sysVar)
+		if err != nil {
+			return err
+		}
 		err = sessionVars.GlobalVarsAccessor.SetGlobalSysVar(name, valStr)
 		if err != nil {
 			return err
@@ -140,6 +140,10 @@ func (e *SetExecutor) setSysVariable(ctx context.Context, name string, v *expres
 		return err
 	}
 	// Set session variable
+	valStr, err := e.getVarValue(v, nil)
+	if err != nil {
+		return err
+	}
 	getSnapshotTSByName := func() uint64 {
 		if name == variable.TiDBSnapshot {
 			return sessionVars.SnapshotTS
@@ -284,11 +288,15 @@ func (e *SetExecutor) setCharset(cs, co string, isSetName bool) error {
 	return errors.Trace(variable.SetSessionSystemVar(sessionVars, variable.CollationConnection, coDb))
 }
 
-func (e *SetExecutor) extractVarValue(v *expression.VarAssignment, sysVar *variable.SysVar) (value string, err error) {
+func (e *SetExecutor) getVarValue(v *expression.VarAssignment, sysVar *variable.SysVar) (value string, err error) {
 	if v.IsDefault {
-		// Set a variable to the compiled-in MySQL default value
-		// http://dev.mysql.com/doc/refman/5.7/en/set-statement.html
-		return sysVar.Value, nil
+		// To set a SESSION variable to the GLOBAL value or a GLOBAL value
+		// to the compiled-in MySQL default value, use the DEFAULT keyword.
+		// See http://dev.mysql.com/doc/refman/5.7/en/set-statement.html
+		if sysVar != nil {
+			return sysVar.Value, nil
+		}
+		return variable.GetGlobalSystemVar(e.ctx.GetSessionVars(), v.Name)
 	}
 	nativeVal, err := v.Expr.Eval(chunk.Row{})
 	if err != nil || nativeVal.IsNull() {

diff --git a/executor/set_test.go b/executor/set_test.go
@@ -111,13 +111,13 @@ 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"))
 	tk.MustExec(`set @@global.low_priority_updates="ON";`)
-	tk.MustExec(`set @@session.low_priority_updates=DEFAULT;`) // It will be set to compiled-in default value.
-	tk.MustQuery(`select @@session.low_priority_updates;`).Check(testkit.Rows("0"))
+	tk.MustExec(`set @@session.low_priority_updates=DEFAULT;`) // It will be set to global var value.
+	tk.MustQuery(`select @@session.low_priority_updates;`).Check(testkit.Rows("1"))
 
 	// For mysql jdbc driver issue.
 	tk.MustQuery(`select @@session.tx_read_only;`).Check(testkit.Rows("0"))
@@ -1394,16 +1394,16 @@ func (s *testSuite5) TestDefaultBehavior(c *C) {
 	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 compiled default
-	tk.MustQuery("SELECT @@default_storage_engine").Check(testkit.Rows("InnoDB"))
+	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("InnoDB InnoDB"))
+	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