From 490af37d1e8812cf19416465696e41454078d383 Mon Sep 17 00:00:00 2001 From: Du Chuan Date: Tue, 3 Jul 2018 17:12:18 +0800 Subject: [PATCH] *: add scope check when get system variables (#6958) --- ast/expressions.go | 2 ++ parser/parser.y | 5 +++-- plan/expression_rewriter.go | 7 +++++++ session/session_test.go | 27 +++++++++++++++++++++++++++ sessionctx/variable/sysvar.go | 14 +++++++------- sessionctx/variable/varsutil.go | 19 +++++++++++++++++++ 6 files changed, 65 insertions(+), 9 deletions(-) diff --git a/ast/expressions.go b/ast/expressions.go index f6356770d5f7c..24e510ff9e5a0 100644 --- a/ast/expressions.go +++ b/ast/expressions.go @@ -944,6 +944,8 @@ type VariableExpr struct { IsGlobal bool // IsSystem indicates whether this variable is a system variable in current session. IsSystem bool + // ExplicitScope indicates whether this variable scope is set explicitly. + ExplicitScope bool // Value is the variable value. Value ExprNode } diff --git a/parser/parser.y b/parser/parser.y index cb7f8dafb8ca8..883b0ef252d9d 100644 --- a/parser/parser.y +++ b/parser/parser.y @@ -5008,6 +5008,7 @@ SystemVariable: { v := strings.ToLower($1) var isGlobal bool + explicitScope := true if strings.HasPrefix(v, "@@global.") { isGlobal = true v = strings.TrimPrefix(v, "@@global.") @@ -5016,9 +5017,9 @@ SystemVariable: } else if strings.HasPrefix(v, "@@local.") { v = strings.TrimPrefix(v, "@@local.") } else if strings.HasPrefix(v, "@@") { - v = strings.TrimPrefix(v, "@@") + v, explicitScope = strings.TrimPrefix(v, "@@"), false } - $$ = &ast.VariableExpr{Name: v, IsGlobal: isGlobal, IsSystem: true} + $$ = &ast.VariableExpr{Name: v, IsGlobal: isGlobal, IsSystem: true, ExplicitScope: explicitScope} } UserVariable: diff --git a/plan/expression_rewriter.go b/plan/expression_rewriter.go index bf389fe379d87..ecedf9519283a 100644 --- a/plan/expression_rewriter.go +++ b/plan/expression_rewriter.go @@ -781,6 +781,13 @@ func (er *expressionRewriter) rewriteVariable(v *ast.VariableExpr) { } var val string var err error + if v.ExplicitScope { + err = variable.ValidateGetSystemVar(name, v.IsGlobal) + if err != nil { + er.err = errors.Trace(err) + return + } + } if v.IsGlobal { val, err = variable.GetGlobalSystemVar(sessionVars, name) } else { diff --git a/session/session_test.go b/session/session_test.go index 7a4c71d2e1825..1e046630af781 100644 --- a/session/session_test.go +++ b/session/session_test.go @@ -371,6 +371,33 @@ func (s *testSessionSuite) TestGlobalVarAccessor(c *C) { tk.MustExec("set @@global.autocommit=1") } +func (s *testSessionSuite) TestGetSysVariables(c *C) { + tk := testkit.NewTestKitWithInit(c, s.store) + + // Test ScopeSession + tk.MustExec("select @@warning_count") + tk.MustExec("select @@session.warning_count") + tk.MustExec("select @@local.warning_count") + _, err := tk.Exec("select @@global.warning_count") + c.Assert(terror.ErrorEqual(err, variable.ErrIncorrectScope), IsTrue) + + // Test ScopeGlobal + tk.MustExec("select @@max_connections") + tk.MustExec("select @@global.max_connections") + _, err = tk.Exec("select @@session.max_connections") + c.Assert(terror.ErrorEqual(err, variable.ErrIncorrectScope), IsTrue) + _, err = tk.Exec("select @@local.max_connections") + c.Assert(terror.ErrorEqual(err, variable.ErrIncorrectScope), IsTrue) + + // Test ScopeNone + tk.MustExec("select @@performance_schema_max_mutex_classes") + tk.MustExec("select @@global.performance_schema_max_mutex_classes") + _, err = tk.Exec("select @@session.performance_schema_max_mutex_classes") + c.Assert(terror.ErrorEqual(err, variable.ErrIncorrectScope), IsTrue) + _, err = tk.Exec("select @@local.performance_schema_max_mutex_classes") + c.Assert(terror.ErrorEqual(err, variable.ErrIncorrectScope), IsTrue) +} + func (s *testSessionSuite) TestRetryResetStmtCtx(c *C) { tk := testkit.NewTestKitWithInit(c, s.store) tk.MustExec("create table retrytxn (a int unique, b int)") diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index 1af501dde1bf3..c2112ccf811c3 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -59,18 +59,18 @@ func GetSysVar(name string) *SysVar { // Variable error codes. const ( CodeUnknownStatusVar terror.ErrCode = 1 - CodeUnknownSystemVar terror.ErrCode = 1193 - CodeIncorrectScope terror.ErrCode = 1238 - CodeUnknownTimeZone terror.ErrCode = 1298 - CodeReadOnly terror.ErrCode = 1621 + CodeUnknownSystemVar terror.ErrCode = mysql.ErrUnknownSystemVariable + CodeIncorrectScope terror.ErrCode = mysql.ErrIncorrectGlobalLocalVar + CodeUnknownTimeZone terror.ErrCode = mysql.ErrUnknownTimeZone + CodeReadOnly terror.ErrCode = mysql.ErrVariableIsReadonly ) // Variable errors var ( UnknownStatusVar = terror.ClassVariable.New(CodeUnknownStatusVar, "unknown status variable") - UnknownSystemVar = terror.ClassVariable.New(CodeUnknownSystemVar, "unknown system variable '%s'") - ErrIncorrectScope = terror.ClassVariable.New(CodeIncorrectScope, "Incorrect variable scope") - ErrUnknownTimeZone = terror.ClassVariable.New(CodeUnknownTimeZone, "unknown or incorrect time zone: %s") + UnknownSystemVar = terror.ClassVariable.New(CodeUnknownSystemVar, mysql.MySQLErrName[mysql.ErrUnknownSystemVariable]) + ErrIncorrectScope = terror.ClassVariable.New(CodeIncorrectScope, mysql.MySQLErrName[mysql.ErrIncorrectGlobalLocalVar]) + ErrUnknownTimeZone = terror.ClassVariable.New(CodeUnknownTimeZone, mysql.MySQLErrName[mysql.ErrUnknownTimeZone]) ErrReadOnly = terror.ClassVariable.New(CodeReadOnly, "variable is read only") ) diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index c94b09335bc18..8e93112bdac50 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -137,6 +137,25 @@ func SetSessionSystemVar(vars *SessionVars, name string, value types.Datum) erro return vars.SetSystemVar(name, sVal) } +// ValidateGetSystemVar checks if system variable exists and validates its scope when get system variable. +func ValidateGetSystemVar(name string, isGlobal bool) error { + sysVar, exists := SysVars[name] + if !exists { + return UnknownSystemVar.GenByArgs(name) + } + switch sysVar.Scope { + case ScopeGlobal, ScopeNone: + if !isGlobal { + return ErrIncorrectScope.GenByArgs(name, "GLOBAL") + } + case ScopeSession: + if isGlobal { + return ErrIncorrectScope.GenByArgs(name, "SESSION") + } + } + return nil +} + // TiDBOptOn could be used for all tidb session variable options, we use "ON"/1 to turn on those options. func TiDBOptOn(opt string) bool { return strings.EqualFold(opt, "ON") || opt == "1"