sessionctx, executor: Add correctness check when set system variables (…

…#7117)

executor/ddl_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/pingcap/tidb/plan"
 	"github.com/pingcap/tidb/sessionctx/variable"
 	"github.com/pingcap/tidb/table"
+	"github.com/pingcap/tidb/terror"
 	"github.com/pingcap/tidb/types"
 	"github.com/pingcap/tidb/util/chunk"
 	"github.com/pingcap/tidb/util/testkit"
@@ -409,17 +410,15 @@ func (s *testSuite) TestSetDDLReorgWorkerCnt(c *C) {
 	c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(1))
 	tk.MustExec("set tidb_ddl_reorg_worker_cnt = 100")
 	c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(100))
-	tk.MustExec("set tidb_ddl_reorg_worker_cnt = invalid_val")
-	c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(variable.DefTiDBDDLReorgWorkerCount))
+	_, err := tk.Exec("set tidb_ddl_reorg_worker_cnt = invalid_val")
+	c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue)
 	tk.MustExec("set tidb_ddl_reorg_worker_cnt = 100")
 	c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(100))
-	tk.MustExec("set tidb_ddl_reorg_worker_cnt = -1")
-	c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(variable.DefTiDBDDLReorgWorkerCount))
+	_, err = tk.Exec("set tidb_ddl_reorg_worker_cnt = -1")
+	c.Assert(terror.ErrorEqual(err, variable.ErrWrongValueForVar), IsTrue)
 
-	res := tk.MustQuery("select @@tidb_ddl_reorg_worker_cnt")
-	res.Check(testkit.Rows("-1"))
 	tk.MustExec("set tidb_ddl_reorg_worker_cnt = 100")
-	res = tk.MustQuery("select @@tidb_ddl_reorg_worker_cnt")
+	res := tk.MustQuery("select @@tidb_ddl_reorg_worker_cnt")
 	res.Check(testkit.Rows("100"))
 
 	res = tk.MustQuery("select @@global.tidb_ddl_reorg_worker_cnt")

executor/set_test.go

@@ -17,7 +17,9 @@ import (
 	. "github.com/pingcap/check"
 	"github.com/pingcap/tidb/sessionctx"
 	"github.com/pingcap/tidb/sessionctx/variable"
+	"github.com/pingcap/tidb/terror"
 	"github.com/pingcap/tidb/util/testkit"
+	"github.com/pingcap/tidb/util/testutil"
 	"golang.org/x/net/context"
 )
 
@@ -88,16 +90,16 @@ func (s *testSuite) TestSetVar(c *C) {
 	// Set default
 	// {ScopeGlobal | ScopeSession, "low_priority_updates", "OFF"},
 	// For global var
-	tk.MustQuery(`select @@global.low_priority_updates;`).Check(testkit.Rows("OFF"))
+	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("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.MustQuery(`select @@global.low_priority_updates;`).Check(testkit.Rows("OFF"))
+	tk.MustQuery(`select @@global.low_priority_updates;`).Check(testkit.Rows("0"))
 	// For session
-	tk.MustQuery(`select @@session.low_priority_updates;`).Check(testkit.Rows("OFF"))
+	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 global var value.
-	tk.MustQuery(`select @@session.low_priority_updates;`).Check(testkit.Rows("ON"))
+	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"))
@@ -212,15 +214,15 @@ func (s *testSuite) TestSetVar(c *C) {
 	tk.MustQuery("select @@session.tx_isolation").Check(testkit.Rows("READ-COMMITTED"))
 
 	tk.MustExec("set global avoid_temporal_upgrade = on")
-	tk.MustQuery(`select @@global.avoid_temporal_upgrade;`).Check(testkit.Rows("ON"))
+	tk.MustQuery(`select @@global.avoid_temporal_upgrade;`).Check(testkit.Rows("1"))
 	tk.MustExec("set @@global.avoid_temporal_upgrade = off")
-	tk.MustQuery(`select @@global.avoid_temporal_upgrade;`).Check(testkit.Rows("off"))
+	tk.MustQuery(`select @@global.avoid_temporal_upgrade;`).Check(testkit.Rows("0"))
 	tk.MustExec("set session sql_log_bin = on")
-	tk.MustQuery(`select @@session.sql_log_bin;`).Check(testkit.Rows("ON"))
+	tk.MustQuery(`select @@session.sql_log_bin;`).Check(testkit.Rows("1"))
 	tk.MustExec("set sql_log_bin = off")
-	tk.MustQuery(`select @@session.sql_log_bin;`).Check(testkit.Rows("off"))
+	tk.MustQuery(`select @@session.sql_log_bin;`).Check(testkit.Rows("0"))
 	tk.MustExec("set @@sql_log_bin = on")
-	tk.MustQuery(`select @@session.sql_log_bin;`).Check(testkit.Rows("ON"))
+	tk.MustQuery(`select @@session.sql_log_bin;`).Check(testkit.Rows("1"))
 }
 
 func (s *testSuite) TestSetCharset(c *C) {
@@ -247,3 +249,66 @@ func (s *testSuite) TestSetCharset(c *C) {
 	// Issue 1523
 	tk.MustExec(`SET NAMES binary`)
 }
+
+func (s *testSuite) TestValidateSetVar(c *C) {
+	tk := testkit.NewTestKit(c, s.store)
+
+	_, err := tk.Exec("set global tidb_distsql_scan_concurrency='fff';")
+	c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue)
+
+	_, err = tk.Exec("set global tidb_distsql_scan_concurrency=-1;")
+	c.Assert(terror.ErrorEqual(err, variable.ErrWrongValueForVar), IsTrue)
+
+	_, err = tk.Exec("set @@tidb_distsql_scan_concurrency='fff';")
+	c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue)
+
+	_, err = tk.Exec("set @@tidb_distsql_scan_concurrency=-1;")
+	c.Assert(terror.ErrorEqual(err, variable.ErrWrongValueForVar), IsTrue)
+
+	_, err = tk.Exec("set @@tidb_batch_delete='ok';")
+	c.Assert(terror.ErrorEqual(err, variable.ErrWrongValueForVar), IsTrue)
+
+	tk.MustExec("set @@tidb_batch_delete='On';")
+	tk.MustExec("set @@tidb_batch_delete='oFf';")
+	tk.MustExec("set @@tidb_batch_delete=1;")
+	tk.MustExec("set @@tidb_batch_delete=0;")
+
+	_, err = tk.Exec("set @@tidb_batch_delete=3;")
+	c.Assert(terror.ErrorEqual(err, variable.ErrWrongValueForVar), IsTrue)
+
+	_, err = tk.Exec("set @@tidb_mem_quota_mergejoin='tidb';")
+	c.Assert(terror.ErrorEqual(err, variable.ErrWrongValueForVar), IsTrue)
+
+	tk.MustExec("set @@group_concat_max_len=1")
+	tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect group_concat_max_len value: '1'"))
+	result := tk.MustQuery("select @@group_concat_max_len;")
+	result.Check(testkit.Rows("4"))
+
+	_, err = tk.Exec("set @@group_concat_max_len = 18446744073709551616")
+	c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue)
+
+	// Test illegal type
+	_, err = tk.Exec("set @@group_concat_max_len='hello'")
+	c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue)
+
+	tk.MustExec("set @@default_week_format=-1")
+	tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect default_week_format value: '-1'"))
+	result = tk.MustQuery("select @@default_week_format;")
+	result.Check(testkit.Rows("0"))
+
+	tk.MustExec("set @@default_week_format=9")
+	tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect default_week_format value: '9'"))
+	result = tk.MustQuery("select @@default_week_format;")
+	result.Check(testkit.Rows("7"))
+
+	_, err = tk.Exec("set @@error_count = 0")
+	c.Assert(terror.ErrorEqual(err, variable.ErrReadOnly), IsTrue)
+
+	_, err = tk.Exec("set @@warning_count = 0")
+	c.Assert(terror.ErrorEqual(err, variable.ErrReadOnly), IsTrue)
+
+	tk.MustExec("set time_zone='SySTeM'")
+	result = tk.MustQuery("select @@time_zone;")
+	result.Check(testkit.Rows("SYSTEM"))
+
+}

session/session.go

@@ -689,16 +689,22 @@ func (s *session) GetGlobalSysVar(name string) (string, error) {
 }
 
 // SetGlobalSysVar implements GlobalVarAccessor.SetGlobalSysVar interface.
-func (s *session) SetGlobalSysVar(name string, value string) error {
+func (s *session) SetGlobalSysVar(name, value string) error {
 	if name == variable.SQLModeVar {
 		value = mysql.FormatSQLModeStr(value)
 		if _, err := mysql.GetSQLMode(value); err != nil {
 			return errors.Trace(err)
 		}
 	}
+	var sVal string
+	var err error
+	sVal, err = variable.ValidateSetSystemVar(s.sessionVars, name, value)
+	if err != nil {
+		return errors.Trace(err)
+	}
 	sql := fmt.Sprintf(`REPLACE %s.%s VALUES ('%s', '%s');`,
-		mysql.SystemDB, mysql.GlobalVariablesTable, strings.ToLower(name), value)
-	_, _, err := s.ExecRestrictedSQL(s, sql)
+		mysql.SystemDB, mysql.GlobalVariablesTable, strings.ToLower(name), sVal)
+	_, _, err = s.ExecRestrictedSQL(s, sql)
 	return errors.Trace(err)
 }
 

sessionctx/variable/session.go

@@ -545,8 +545,7 @@ func (s *SessionVars) SetSystemVar(name string, val string) error {
 	case TiDBEnableTablePartition:
 		s.EnableTablePartition = TiDBOptOn(val)
 	case TiDBDDLReorgWorkerCount:
-		workerCnt := tidbOptPositiveInt32(val, DefTiDBDDLReorgWorkerCount)
-		SetDDLReorgWorkerCounter(int32(workerCnt))
+		SetDDLReorgWorkerCounter(int32(tidbOptPositiveInt32(val, DefTiDBDDLReorgWorkerCount)))
 	}
 	s.systems[name] = val
 	return nil