Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: add support for disabling noop variables #35496

Merged
merged 8 commits into from
Jun 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions errno/errcode.go
Original file line number Diff line number Diff line change
Expand Up @@ -1023,6 +1023,8 @@ const (
ErrAssertionFailed = 8141
ErrInstanceScope = 8142
ErrNonTransactionalJobFailure = 8143
ErrSettingNoopVariable = 8144
ErrGettingNoopVariable = 8145

// Error codes used by TiDB ddl package
ErrUnsupportedDDLOperation = 8200
Expand Down
2 changes: 2 additions & 0 deletions errno/errname.go
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,8 @@ var MySQLErrName = map[uint16]*mysql.ErrMessage{
ErrAssertionFailed: mysql.Message("assertion failed: key: %s, assertion: %s, start_ts: %v, existing start ts: %v, existing commit ts: %v", []int{0}),
ErrInstanceScope: mysql.Message("modifying %s will require SET GLOBAL in a future version of TiDB", nil),
ErrNonTransactionalJobFailure: mysql.Message("non-transactional job failed, job id: %d, total jobs: %d. job range: [%s, %s], job sql: %s, err: %v", []int{2, 3, 4}),
ErrSettingNoopVariable: mysql.Message("setting %s has no effect in TiDB", nil),
ErrGettingNoopVariable: mysql.Message("variable %s has no effect in TiDB", nil),
xhebox marked this conversation as resolved.
Show resolved Hide resolved

ErrWarnOptimizerHintInvalidInteger: mysql.Message("integer value is out of range in '%s'", nil),
ErrWarnOptimizerHintUnsupportedHint: mysql.Message("Optimizer hint %s is not supported by TiDB and is ignored", nil),
Expand Down
10 changes: 10 additions & 0 deletions errors.toml
Original file line number Diff line number Diff line change
Expand Up @@ -1456,6 +1456,11 @@ error = '''
modifying %s will require SET GLOBAL in a future version of TiDB
'''

["executor:8144"]
error = '''
setting %s has no effect in TiDB
'''

["executor:8212"]
error = '''
Failed to split region ranges: %s
Expand Down Expand Up @@ -2076,6 +2081,11 @@ error = '''
Column '%s' in ANALYZE column option does not exist in table '%s'
'''

["planner:8145"]
error = '''
variable %s has no effect in TiDB
'''

["planner:8242"]
error = '''
'%s' is unsupported on cache tables.
Expand Down
1 change: 1 addition & 0 deletions executor/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ var (
ErrInvalidSplitRegionRanges = dbterror.ClassExecutor.NewStd(mysql.ErrInvalidSplitRegionRanges)
ErrViewInvalid = dbterror.ClassExecutor.NewStd(mysql.ErrViewInvalid)
ErrInstanceScope = dbterror.ClassExecutor.NewStd(mysql.ErrInstanceScope)
ErrSettingNoopVariable = dbterror.ClassExecutor.NewStd(mysql.ErrSettingNoopVariable)

ErrBRIEBackupFailed = dbterror.ClassExecutor.NewStd(mysql.ErrBRIEBackupFailed)
ErrBRIERestoreFailed = dbterror.ClassExecutor.NewStd(mysql.ErrBRIERestoreFailed)
Expand Down
7 changes: 6 additions & 1 deletion executor/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,12 @@ func (e *SetExecutor) setSysVariable(ctx context.Context, name string, v *expres
}
return variable.ErrUnknownSystemVar.GenWithStackByArgs(name)
}

if sysVar.IsNoop && !variable.EnableNoopVariables.Load() {
// The variable is a noop. For compatibility we allow it to still
// be changed, but we append a warning since users might be expecting
// something that's not going to happen.
sessionVars.StmtCtx.AppendWarning(ErrSettingNoopVariable.GenWithStackByArgs(sysVar.Name))
}
if sysVar.HasInstanceScope() && !v.IsGlobal && sessionVars.EnableLegacyInstanceScope {
// For backward compatibility we will change the v.IsGlobal to true,
// and append a warning saying this will not be supported in future.
Expand Down
25 changes: 25 additions & 0 deletions executor/set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,31 @@ func TestSetVar(t *testing.T) {
tk.MustQuery("select @@tidb_cost_model_version").Check(testkit.Rows("2"))
}

func TestGetSetNoopVars(t *testing.T) {
store, clean := testkit.CreateMockStore(t)
defer clean()
tk := testkit.NewTestKit(t, store)

// By default you can get/set noop sysvars without issue.
tk.MustQuery("SELECT @@query_cache_type").Check(testkit.Rows("OFF"))
tk.MustQuery("SHOW VARIABLES LIKE 'query_cache_type'").Check(testkit.Rows("query_cache_type OFF"))
tk.MustExec("SET query_cache_type=2")
tk.MustQuery("SELECT @@query_cache_type").Check(testkit.Rows("DEMAND"))
// When tidb_enable_noop_variables is OFF, you can GET in @@ context
// and always SET. But you can't see in SHOW VARIABLES.
// Warnings are also returned.
tk.MustExec("SET GLOBAL tidb_enable_noop_variables = OFF")
defer tk.MustExec("SET GLOBAL tidb_enable_noop_variables = ON")
morgo marked this conversation as resolved.
Show resolved Hide resolved
tk.MustQuery("SELECT @@global.tidb_enable_noop_variables").Check(testkit.Rows("OFF"))
tk.MustQuery("SELECT @@query_cache_type").Check(testkit.Rows("DEMAND"))
tk.MustQuery("SHOW WARNINGS").Check(testkit.Rows("Warning 8145 variable query_cache_type has no effect in TiDB"))
tk.MustQuery("SHOW VARIABLES LIKE 'query_cache_type'").Check(testkit.Rows())
tk.MustExec("SET query_cache_type = OFF")
tk.MustQuery("SHOW WARNINGS").Check(testkit.Rows("Warning 8144 setting query_cache_type has no effect in TiDB"))
// but the change is still effective.
tk.MustQuery("SELECT @@query_cache_type").Check(testkit.Rows("OFF"))
}

func TestTruncateIncorrectIntSessionVar(t *testing.T) {
store, clean := testkit.CreateMockStore(t)
defer clean()
Expand Down
6 changes: 6 additions & 0 deletions executor/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,9 @@ func (e *ShowExec) fetchShowVariables() (err error) {
// otherwise, fetch the value from table `mysql.Global_Variables`.
for _, v := range variable.GetSysVars() {
if v.Scope != variable.ScopeSession {
if v.IsNoop && !variable.EnableNoopVariables.Load() {
continue
}
if fieldFilter != "" && v.Name != fieldFilter {
continue
} else if fieldPatternsLike != nil && !fieldPatternsLike.DoMatch(v.Name) {
Expand All @@ -842,6 +845,9 @@ func (e *ShowExec) fetchShowVariables() (err error) {
// 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 v.IsNoop && !variable.EnableNoopVariables.Load() {
continue
}
if fieldFilter != "" && v.Name != fieldFilter {
continue
} else if fieldPatternsLike != nil && !fieldPatternsLike.DoMatch(v.Name) {
Expand Down
1 change: 1 addition & 0 deletions planner/core/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,4 +109,5 @@ var (
ErrViewSelectTemporaryTable = dbterror.ClassOptimizer.NewStd(mysql.ErrViewSelectTmptable)
ErrSubqueryMoreThan1Row = dbterror.ClassOptimizer.NewStd(mysql.ErrSubqueryNo1Row)
ErrKeyPart0 = dbterror.ClassOptimizer.NewStd(mysql.ErrKeyPart0)
ErrGettingNoopVariable = dbterror.ClassOptimizer.NewStd(mysql.ErrGettingNoopVariable)
)
4 changes: 4 additions & 0 deletions planner/core/expression_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -1297,6 +1297,10 @@ func (er *expressionRewriter) rewriteVariable(v *ast.VariableExpr) {
}
return
}
if sysVar.IsNoop && !variable.EnableNoopVariables.Load() {
// The variable does nothing, append a warning to the statement output.
sessionVars.StmtCtx.AppendWarning(ErrGettingNoopVariable.GenWithStackByArgs(sysVar.Name))
}
if sem.IsEnabled() && sem.IsInvisibleSysVar(sysVar.Name) {
err := ErrSpecificAccessDenied.GenWithStackByArgs("RESTRICTED_VARIABLES_ADMIN")
er.b.visitInfo = appendDynamicVisitInfo(er.b.visitInfo, "RESTRICTED_VARIABLES_ADMIN", false, err)
Expand Down
6 changes: 6 additions & 0 deletions sessionctx/variable/sysvar.go
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,12 @@ var defaultSysVars = []*SysVar{
}, GetGlobal: func(s *SessionVars) (string, error) {
return BoolToOnOff(EnableConcurrentDDL.Load()), nil
}},
{Scope: ScopeGlobal, Name: TiDBEnableNoopVariables, Value: BoolToOnOff(DefTiDBEnableNoopVariables), Type: TypeEnum, PossibleValues: []string{Off, On, Warn}, SetGlobal: func(s *SessionVars, val string) error {
EnableNoopVariables.Store(TiDBOptOn(val))
return nil
}, GetGlobal: func(s *SessionVars) (string, error) {
return BoolToOnOff(EnableNoopVariables.Load()), nil
}},

/* The system variables below have GLOBAL and SESSION scope */
{Scope: ScopeGlobal | ScopeSession, Name: SQLSelectLimit, Value: "18446744073709551615", Type: TypeUnsigned, MinValue: 0, MaxValue: math.MaxUint64, SetSession: func(s *SessionVars, val string) error {
Expand Down
5 changes: 5 additions & 0 deletions sessionctx/variable/tidb_vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,9 @@ const (
// TiDBQueryLogMaxLen is used to set the max length of the query in the log.
TiDBQueryLogMaxLen = "tidb_query_log_max_len"

// TiDBEnableNoopVariables is used to indicate if noops appear in SHOW [GLOBAL] VARIABLES
TiDBEnableNoopVariables = "tidb_enable_noop_variables"

// TiDBNonTransactionalIgnoreError is used to ignore error in non-transactional DMLs.
// When set to false, a non-transactional DML returns when it meets the first error.
// When set to true, a non-transactional DML finishes all batches even if errors are met in some batches.
Expand Down Expand Up @@ -855,6 +858,7 @@ const (
DefTiDBWaitSplitRegionFinish = true
DefWaitSplitRegionTimeout = 300 // 300s
DefTiDBEnableNoopFuncs = Off
DefTiDBEnableNoopVariables = true
DefTiDBAllowRemoveAutoInc = false
DefTiDBUsePlanBaselines = true
DefTiDBEvolvePlanBaselines = false
Expand Down Expand Up @@ -985,6 +989,7 @@ var (
PreparedPlanCacheSize = atomic.NewUint64(DefTiDBPrepPlanCacheSize)
PreparedPlanCacheMemoryGuardRatio = atomic.NewFloat64(DefTiDBPrepPlanCacheMemoryGuardRatio)
EnableConcurrentDDL = atomic.NewBool(DefTiDBEnableConcurrentDDL)
EnableNoopVariables = atomic.NewBool(DefTiDBEnableNoopVariables)
)

var (
Expand Down