sysvar: bypass validation for noop variables (#31566)

close #31538
pingcap · Jan 11, 2022 · 1ffd6c0 · 1ffd6c0
1 parent 706abd6
commit 1ffd6c0
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 2 deletions.
diff --git a/expression/integration_serial_test.go b/expression/integration_serial_test.go
@@ -3851,8 +3851,8 @@ func TestSetVariables(t *testing.T) {
 	tk.MustExec("set global tidb_enable_noop_functions=1")
 
 	_, err = tk.Exec("set @@global.max_user_connections='';")
-	require.Error(t, err)
-	require.Error(t, err, variable.ErrWrongTypeForVar.GenWithStackByArgs("max_user_connections").Error())
+	require.NoError(t, err)
+	//require.Error(t, err, variable.ErrWrongTypeForVar.GenWithStackByArgs("max_user_connections").Error())
 	_, err = tk.Exec("set @@global.max_prepared_stmt_count='';")
 	require.Error(t, err)
 	require.Error(t, err, variable.ErrWrongTypeForVar.GenWithStackByArgs("max_prepared_stmt_count").Error())

diff --git a/sessionctx/variable/sysvar_test.go b/sessionctx/variable/sysvar_test.go
@@ -834,3 +834,16 @@ func TestIndexMergeSwitcher(t *testing.T) {
 	require.Equal(t, DefTiDBEnableIndexMerge, true)
 	require.Equal(t, BoolToOnOff(DefTiDBEnableIndexMerge), val)
 }
+
+func TestNoValidateForNoop(t *testing.T) {
+	vars := NewSessionVars()
+
+	// for noop variables, no error
+	val, err := GetSysVar("rpl_semi_sync_slave_enabled").ValidateFromType(vars, "", ScopeGlobal)
+	require.NoError(t, err)
+	require.Equal(t, val, "")
+
+	// for other variables, error
+	_, err = GetSysVar(TiDBAllowBatchCop).ValidateFromType(vars, "", ScopeGlobal)
+	require.Error(t, err)
+}
diff --git a/sessionctx/variable/variable.go b/sessionctx/variable/variable.go
@@ -261,6 +261,10 @@ 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) {
+	// TODO: this is a temporary solution for issue: https://github.com/pingcap/tidb/issues/31538, an elegant solution is needed.
+	if value == "" && sv.IsNoop {
+		return 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.