Revert "executor: fix show global variables return session variables …

…also (#19341)" (#26258)
pingcap · Sep 3, 2021 · 14ac993 · 14ac993
1 parent 8ca7563
commit 14ac993
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 49 deletions.
diff --git a/executor/show.go b/executor/show.go
@@ -662,42 +662,52 @@ func (e *ShowExec) fetchShowMasterStatus() error {
 
 func (e *ShowExec) fetchShowVariables() (err error) {
 	var (
-		value       string
-		sessionVars = e.ctx.GetSessionVars()
+		value         string
+		ok            bool
+		sessionVars   = e.ctx.GetSessionVars()
+		unreachedVars = make([]string, 0, len(variable.GetSysVars()))
 	)
-	if e.GlobalScope {
-		// Collect global scope variables,
-		// 1. Exclude the variables of ScopeSession in variable.SysVars;
-		// 2. If the variable is ScopeNone, it's a read-only variable, return the default value of it,
-		// 		otherwise, fetch the value from table `mysql.Global_Variables`.
-		for _, v := range variable.GetSysVars() {
-			if v.Scope != variable.ScopeSession {
-				if variable.FilterImplicitFeatureSwitch(v) {
-					continue
-				}
-				value, err = variable.GetGlobalSystemVar(sessionVars, v.Name)
-				if err != nil {
-					return errors.Trace(err)
-				}
-				e.appendRow([]interface{}{v.Name, value})
-			}
-		}
-		return nil
-	}
-
-	// Collect session scope variables,
-	// If it is a session only variable, use the default value defined in code,
-	//   otherwise, fetch the value from table `mysql.Global_Variables`.
 	for _, v := range variable.GetSysVars() {
-		if variable.FilterImplicitFeatureSwitch(v) {
+		if variable.FilterImplicitFeatureSwitch(v.Name) {
 			continue
 		}
-		value, err = variable.GetSessionSystemVar(sessionVars, v.Name)
+		if !e.GlobalScope {
+			// For a session scope variable,
+			// 1. try to fetch value from SessionVars.Systems;
+			// 2. if this variable is session-only, fetch value from SysVars
+			//		otherwise, fetch the value from table `mysql.Global_Variables`.
+			value, ok, err = variable.GetSessionOnlySysVars(sessionVars, v.Name)
+		} else {
+			// If the scope of a system variable is ScopeNone,
+			// it's a read-only variable, so we return the default value of it.
+			// Otherwise, we have to fetch the values from table `mysql.Global_Variables` for global variable names.
+			value, ok, err = variable.GetScopeNoneSystemVar(v.Name)
+		}
 		if err != nil {
 			return errors.Trace(err)
 		}
+		if !ok {
+			unreachedVars = append(unreachedVars, v.Name)
+			continue
+		}
 		e.appendRow([]interface{}{v.Name, value})
 	}
+	if len(unreachedVars) != 0 {
+		systemVars, err := sessionVars.GlobalVarsAccessor.GetAllSysVars()
+		if err != nil {
+			return errors.Trace(err)
+		}
+		for _, varName := range unreachedVars {
+			if variable.FilterImplicitFeatureSwitch(varName) {
+				continue
+			}
+			varValue, ok := systemVars[varName]
+			if !ok {
+				varValue = variable.GetSysVar(varName).Value
+			}
+			e.appendRow([]interface{}{varName, varValue})
+		}
+	}
 	return nil
 }
 

diff --git a/executor/show_test.go b/executor/show_test.go
@@ -1242,11 +1242,10 @@ func (s *testSerialSuite1) TestShowCreateTableWithIntegerDisplayLengthWarnings(c
 
 func (s *testSuite5) TestShowVar(c *C) {
 	tk := testkit.NewTestKit(c, s.store)
-	var showSQL string
 	sessionVars := make([]string, 0, len(variable.GetSysVars()))
 	globalVars := make([]string, 0, len(variable.GetSysVars()))
 	for _, v := range variable.GetSysVars() {
-		if variable.FilterImplicitFeatureSwitch(v) {
+		if variable.FilterImplicitFeatureSwitch(v.Name) {
 			continue
 		}
 
@@ -1257,23 +1256,6 @@ func (s *testSuite5) TestShowVar(c *C) {
 		}
 	}
 
-	// When ScopeSession only. `show global variables` must return empty.
-	sessionVarsStr := strings.Join(sessionVars, "','")
-	showSQL = "show variables where variable_name in('" + sessionVarsStr + "')"
-	res := tk.MustQuery(showSQL)
-	c.Check(res.Rows(), HasLen, len(sessionVars))
-	showSQL = "show global variables where variable_name in('" + sessionVarsStr + "')"
-	res = tk.MustQuery(showSQL)
-	c.Check(res.Rows(), HasLen, 0)
-
-	globalVarsStr := strings.Join(globalVars, "','")
-	showSQL = "show variables where variable_name in('" + globalVarsStr + "')"
-	res = tk.MustQuery(showSQL)
-	c.Check(res.Rows(), HasLen, len(globalVars))
-	showSQL = "show global variables where variable_name in('" + globalVarsStr + "')"
-	res = tk.MustQuery(showSQL)
-	c.Check(res.Rows(), HasLen, len(globalVars))
-
 	// Test for switch variable which shouldn't seen by users.
 	for _, one := range variable.FeatureSwitchVariables {
 		res := tk.MustQuery("show variables like '" + one + "'")

diff --git a/session/session.go b/session/session.go
@@ -933,6 +933,33 @@ func (s *session) varFromTiDBTable(name string) bool {
 	return false
 }
 
+// GetAllSysVars implements GlobalVarAccessor.GetAllSysVars interface.
+func (s *session) GetAllSysVars() (map[string]string, error) {
+	if s.Value(sessionctx.Initing) != nil {
+		return nil, nil
+	}
+	stmt, err := s.ParseWithParams(context.TODO(), `SELECT VARIABLE_NAME, VARIABLE_VALUE FROM %n.%n`, mysql.SystemDB, mysql.GlobalVariablesTable)
+	if err != nil {
+		return nil, err
+	}
+	rows, _, err := s.ExecRestrictedStmt(context.TODO(), stmt)
+	if err != nil {
+		return nil, err
+	}
+	ret := make(map[string]string, len(rows))
+	for _, r := range rows {
+		k, v := r.GetString(0), r.GetString(1)
+		if s.varFromTiDBTable(k) {
+			if v, err = s.getTiDBTableValue(k, v); err == nil {
+				ret[k] = v
+			}
+		} else {
+			ret[k] = v
+		}
+	}
+	return ret, nil
+}
+
 // GetGlobalSysVar implements GlobalVarAccessor.GetGlobalSysVar interface.
 func (s *session) GetGlobalSysVar(name string) (string, error) {
 	if name == variable.TiDBSlowLogMasking {

diff --git a/sessionctx/variable/mock_globalaccessor.go b/sessionctx/variable/mock_globalaccessor.go
@@ -22,7 +22,12 @@ func NewMockGlobalAccessor() *MockGlobalAccessor {
 	return new(MockGlobalAccessor)
 }
 
-// GetGlobalSysVar implements GlobalVarAccessor.GetGlobalSysVar interface.
+// GetAllSysVars implements GlobalVarAccessor.GetAllSysVars interface.
+func (m *MockGlobalAccessor) GetAllSysVars() (map[string]string, error) {
+	panic("not supported")
+}
+
+// GetGlobalSysVar implements (s *session) GetAllSysVars().
 func (m *MockGlobalAccessor) GetGlobalSysVar(name string) (string, error) {
 	v, ok := sysVars[name]
 	if ok {

diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go
@@ -1114,6 +1114,8 @@ const (
 
 // GlobalVarAccessor is the interface for accessing global scope system and status variables.
 type GlobalVarAccessor interface {
+	// GetAllSysVars gets all the global system variable values.
+	GetAllSysVars() (map[string]string, error)
 	// GetGlobalSysVar gets the global system variable value for name.
 	GetGlobalSysVar(name string) (string, error)
 	// SetGlobalSysVar sets the global system variable name to value.

diff --git a/sessionctx/variable/tidb_vars.go b/sessionctx/variable/tidb_vars.go
@@ -742,9 +742,9 @@ var FeatureSwitchVariables = []string{
 }
 
 // FilterImplicitFeatureSwitch is used to filter result of show variables, these switches should be turn blind to users.
-func FilterImplicitFeatureSwitch(sysVar *SysVar) bool {
+func FilterImplicitFeatureSwitch(varName string) bool {
 	for _, one := range FeatureSwitchVariables {
-		if one == sysVar.Name {
+		if one == varName {
 			return true
 		}
 	}