From 427762bba19e2a3d25bbc8c2c819584b2ff1a9d8 Mon Sep 17 00:00:00 2001 From: marsishandsome Date: Mon, 8 Apr 2019 15:04:18 +0800 Subject: [PATCH 01/12] add strict compatibility variable --- sessionctx/variable/session.go | 9 ++++++--- sessionctx/variable/sysvar.go | 1 + sessionctx/variable/tidb_vars.go | 5 +++++ sessionctx/variable/varsutil.go | 10 +++++++--- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 6209631c79175..6b546ff64d7ca 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -601,9 +601,12 @@ func (s *SessionVars) WithdrawAllPreparedStmt() { func (s *SessionVars) SetSystemVar(name string, val string) error { switch name { case TxnIsolationOneShot: - switch val { - case "SERIALIZABLE", "READ-UNCOMMITTED": - return ErrUnsupportedValueForVar.GenWithStackByArgs(name, val) + strictCompatibility, e := GetSessionSystemVar(s, TiDBStrictCompatibility) + if !TiDBOptOn(strictCompatibility) || e != nil { + switch val { + case "SERIALIZABLE", "READ-UNCOMMITTED": + return ErrUnsupportedValueForVar.GenWithStackByArgs(name, val) + } } s.TxnIsolationLevelOneShot.State = 1 s.TxnIsolationLevelOneShot.Value = val diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index a4f471054dead..76e12488be3c2 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -673,6 +673,7 @@ var defaultSysVars = []*SysVar{ {ScopeSession, TiDBOptimizerSelectivityLevel, strconv.Itoa(DefTiDBOptimizerSelectivityLevel)}, {ScopeGlobal | ScopeSession, TiDBEnableWindowFunction, BoolToIntStr(DefEnableWindowFunction)}, {ScopeGlobal | ScopeSession, TiDBEnableFastAnalyze, BoolToIntStr(DefTiDBUseFastAnalyze)}, + {ScopeGlobal | ScopeSession, TiDBStrictCompatibility, BoolToIntStr(DefTiDBStrictCompatibility)}, /* The following variable is defined as session scope but is actually server scope. */ {ScopeSession, TiDBGeneralLog, strconv.Itoa(DefTiDBGeneralLog)}, {ScopeSession, TiDBSlowLogThreshold, strconv.Itoa(logutil.DefaultSlowThreshold)}, diff --git a/sessionctx/variable/tidb_vars.go b/sessionctx/variable/tidb_vars.go index a9544c65a0462..d2bf990cb7112 100644 --- a/sessionctx/variable/tidb_vars.go +++ b/sessionctx/variable/tidb_vars.go @@ -131,6 +131,10 @@ const ( // TiDBCheckMb4ValueInUTF8 is used to control whether to enable the check wrong utf8 value. TiDBCheckMb4ValueInUTF8 = "tidb_check_mb4_value_in_utf8" + + // tidb_strict_compatibility is used to control whether to return error when set unsupported transaction + // isolation level. + TiDBStrictCompatibility = "tidb_strict_compatibility" ) // TiDB system variable names that both in session and global scope. @@ -295,6 +299,7 @@ const ( DefEnableWindowFunction = false DefTiDBDDLSlowOprThreshold = 300 DefTiDBUseFastAnalyze = false + DefTiDBStrictCompatibility = false ) // Process global variables. diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index 90cbb54dd6034..eb9139176b0a0 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -427,9 +427,13 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, if !exists { return "", ErrWrongValueForVar.GenWithStackByArgs(name, value) } - switch upVal { - case "SERIALIZABLE", "READ-UNCOMMITTED": - return "", ErrUnsupportedValueForVar.GenWithStackByArgs(name, value) + + strictCompatibility, e := GetSessionSystemVar(vars, TiDBStrictCompatibility) + if !TiDBOptOn(strictCompatibility) || e != nil { + switch upVal { + case "SERIALIZABLE", "READ-UNCOMMITTED": + return "", ErrUnsupportedValueForVar.GenWithStackByArgs(name, value) + } } return upVal, nil case TiDBInitChunkSize: From 3270cf57d8c25c0e0b7492972497cd7e32d8d46c Mon Sep 17 00:00:00 2001 From: marsishandsome Date: Mon, 8 Apr 2019 17:20:38 +0800 Subject: [PATCH 02/12] e -> err --- sessionctx/variable/session.go | 4 ++-- sessionctx/variable/varsutil.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 6b546ff64d7ca..a15eaa330feb3 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -601,8 +601,8 @@ func (s *SessionVars) WithdrawAllPreparedStmt() { func (s *SessionVars) SetSystemVar(name string, val string) error { switch name { case TxnIsolationOneShot: - strictCompatibility, e := GetSessionSystemVar(s, TiDBStrictCompatibility) - if !TiDBOptOn(strictCompatibility) || e != nil { + strictCompatibility, err := GetSessionSystemVar(s, TiDBStrictCompatibility) + if !TiDBOptOn(strictCompatibility) || err != nil { switch val { case "SERIALIZABLE", "READ-UNCOMMITTED": return ErrUnsupportedValueForVar.GenWithStackByArgs(name, val) diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index eb9139176b0a0..2e7cb0cf4431c 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -428,8 +428,8 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, return "", ErrWrongValueForVar.GenWithStackByArgs(name, value) } - strictCompatibility, e := GetSessionSystemVar(vars, TiDBStrictCompatibility) - if !TiDBOptOn(strictCompatibility) || e != nil { + strictCompatibility, err := GetSessionSystemVar(vars, TiDBStrictCompatibility) + if !TiDBOptOn(strictCompatibility) || err != nil { switch upVal { case "SERIALIZABLE", "READ-UNCOMMITTED": return "", ErrUnsupportedValueForVar.GenWithStackByArgs(name, value) From 6579b58760ec9a93b08a459ae0bad3987e5d691a Mon Sep 17 00:00:00 2001 From: marsishandsome Date: Mon, 8 Apr 2019 17:31:09 +0800 Subject: [PATCH 03/12] make fmt --- sessionctx/variable/tidb_vars.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sessionctx/variable/tidb_vars.go b/sessionctx/variable/tidb_vars.go index d2bf990cb7112..7b20c2d979a47 100644 --- a/sessionctx/variable/tidb_vars.go +++ b/sessionctx/variable/tidb_vars.go @@ -299,7 +299,7 @@ const ( DefEnableWindowFunction = false DefTiDBDDLSlowOprThreshold = 300 DefTiDBUseFastAnalyze = false - DefTiDBStrictCompatibility = false + DefTiDBStrictCompatibility = false ) // Process global variables. From 4271cd6fb0b50cf4d58fabea914c7231682c8ad2 Mon Sep 17 00:00:00 2001 From: marsishandsome Date: Mon, 8 Apr 2019 21:43:46 +0800 Subject: [PATCH 04/12] add unit test --- executor/set_test.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/executor/set_test.go b/executor/set_test.go index d7e28bdd271fe..7e9a4d66cc434 100644 --- a/executor/set_test.go +++ b/executor/set_test.go @@ -285,6 +285,41 @@ func (s *testSuite2) TestSetVar(c *C) { c.Assert(err, NotNil) _, err = tk.Exec("set global tidb_batch_commit = 2") c.Assert(err, NotNil) + + // test strict compatibility: init + tk.MustExec("SET GLOBAL tidb_strict_compatibility = 0") + tk.MustExec("SET SESSION tidb_strict_compatibility = 0") + tk.MustExec("SET GLOBAL TRANSACTION ISOLATION LEVEL READ COMMITTED") + tk.MustExec("SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED") + tk.MustQuery("select @@global.tx_isolation").Check(testkit.Rows("READ-COMMITTED")) + tk.MustQuery("select @@global.transaction_isolation").Check(testkit.Rows("READ-COMMITTED")) + tk.MustQuery("select @@session.tx_isolation").Check(testkit.Rows("READ-COMMITTED")) + tk.MustQuery("select @@session.transaction_isolation").Check(testkit.Rows("READ-COMMITTED")) + + // test strict compatibility: error + _, err = tk.Exec("SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE") + c.Assert(terror.ErrorEqual(err, variable.ErrUnsupportedValueForVar), IsTrue, Commentf("err %v", err)) + tk.MustQuery("select @@session.tx_isolation").Check(testkit.Rows("READ-COMMITTED")) + tk.MustQuery("select @@session.transaction_isolation").Check(testkit.Rows("READ-COMMITTED")) + + _, err = tk.Exec("SET GLOBAL TRANSACTION ISOLATION LEVEL SERIALIZABLE") + c.Assert(terror.ErrorEqual(err, variable.ErrUnsupportedValueForVar), IsTrue, Commentf("err %v", err)) + tk.MustQuery("select @@global.tx_isolation").Check(testkit.Rows("READ-COMMITTED")) + tk.MustQuery("select @@global.transaction_isolation").Check(testkit.Rows("READ-COMMITTED")) + + // test strict compatibility: success + tk.MustExec("SET GLOBAL tidb_strict_compatibility = 1") + tk.MustExec("SET SESSION tidb_strict_compatibility = 1") + tk.MustExec("SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE") + tk.MustQuery("select @@session.tx_isolation").Check(testkit.Rows("SERIALIZABLE")) + tk.MustQuery("select @@session.transaction_isolation").Check(testkit.Rows("SERIALIZABLE")) + + // test strict compatibility: success + tk.MustExec("SET GLOBAL tidb_strict_compatibility = 0") + tk.MustExec("SET SESSION tidb_strict_compatibility = 1") + tk.MustExec("SET GLOBAL TRANSACTION ISOLATION LEVEL READ UNCOMMITTED") + tk.MustQuery("select @@global.tx_isolation").Check(testkit.Rows("READ-UNCOMMITTED")) + tk.MustQuery("select @@global.transaction_isolation").Check(testkit.Rows("READ-UNCOMMITTED")) } func (s *testSuite2) TestSetCharset(c *C) { From 8d0a17da3d5e0d116b1998570fef2595d0c0b376 Mon Sep 17 00:00:00 2001 From: marsishandsome Date: Tue, 9 Apr 2019 09:05:28 +0800 Subject: [PATCH 05/12] change tidb_strict_compatibility to tidb_skip_isolation_level_check --- executor/set_test.go | 20 ++++++++++---------- sessionctx/variable/session.go | 14 +++++++++----- sessionctx/variable/sysvar.go | 2 +- sessionctx/variable/tidb_vars.go | 6 +++--- sessionctx/variable/varsutil.go | 15 +++++++++------ 5 files changed, 32 insertions(+), 25 deletions(-) diff --git a/executor/set_test.go b/executor/set_test.go index 7e9a4d66cc434..1480a64798865 100644 --- a/executor/set_test.go +++ b/executor/set_test.go @@ -286,9 +286,9 @@ func (s *testSuite2) TestSetVar(c *C) { _, err = tk.Exec("set global tidb_batch_commit = 2") c.Assert(err, NotNil) - // test strict compatibility: init - tk.MustExec("SET GLOBAL tidb_strict_compatibility = 0") - tk.MustExec("SET SESSION tidb_strict_compatibility = 0") + // test skip isolation level check: init + tk.MustExec("SET GLOBAL tidb_skip_isolation_level_check = 0") + tk.MustExec("SET SESSION tidb_skip_isolation_level_check = 0") tk.MustExec("SET GLOBAL TRANSACTION ISOLATION LEVEL READ COMMITTED") tk.MustExec("SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED") tk.MustQuery("select @@global.tx_isolation").Check(testkit.Rows("READ-COMMITTED")) @@ -296,7 +296,7 @@ func (s *testSuite2) TestSetVar(c *C) { tk.MustQuery("select @@session.tx_isolation").Check(testkit.Rows("READ-COMMITTED")) tk.MustQuery("select @@session.transaction_isolation").Check(testkit.Rows("READ-COMMITTED")) - // test strict compatibility: error + // test skip isolation level check: error _, err = tk.Exec("SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE") c.Assert(terror.ErrorEqual(err, variable.ErrUnsupportedValueForVar), IsTrue, Commentf("err %v", err)) tk.MustQuery("select @@session.tx_isolation").Check(testkit.Rows("READ-COMMITTED")) @@ -307,16 +307,16 @@ func (s *testSuite2) TestSetVar(c *C) { tk.MustQuery("select @@global.tx_isolation").Check(testkit.Rows("READ-COMMITTED")) tk.MustQuery("select @@global.transaction_isolation").Check(testkit.Rows("READ-COMMITTED")) - // test strict compatibility: success - tk.MustExec("SET GLOBAL tidb_strict_compatibility = 1") - tk.MustExec("SET SESSION tidb_strict_compatibility = 1") + // test skip isolation level check: success + tk.MustExec("SET GLOBAL tidb_skip_isolation_level_check = 1") + tk.MustExec("SET SESSION tidb_skip_isolation_level_check = 1") tk.MustExec("SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE") tk.MustQuery("select @@session.tx_isolation").Check(testkit.Rows("SERIALIZABLE")) tk.MustQuery("select @@session.transaction_isolation").Check(testkit.Rows("SERIALIZABLE")) - // test strict compatibility: success - tk.MustExec("SET GLOBAL tidb_strict_compatibility = 0") - tk.MustExec("SET SESSION tidb_strict_compatibility = 1") + // test skip isolation level check: success + tk.MustExec("SET GLOBAL tidb_skip_isolation_level_check = 0") + tk.MustExec("SET SESSION tidb_skip_isolation_level_check = 1") tk.MustExec("SET GLOBAL TRANSACTION ISOLATION LEVEL READ UNCOMMITTED") tk.MustQuery("select @@global.tx_isolation").Check(testkit.Rows("READ-UNCOMMITTED")) tk.MustQuery("select @@global.transaction_isolation").Check(testkit.Rows("READ-UNCOMMITTED")) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index a15eaa330feb3..1e69245f81751 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -17,6 +17,8 @@ import ( "bytes" "crypto/tls" "fmt" + "github.com/pingcap/log" + "go.uber.org/zap" "strconv" "strings" "sync" @@ -601,12 +603,14 @@ func (s *SessionVars) WithdrawAllPreparedStmt() { func (s *SessionVars) SetSystemVar(name string, val string) error { switch name { case TxnIsolationOneShot: - strictCompatibility, err := GetSessionSystemVar(s, TiDBStrictCompatibility) - if !TiDBOptOn(strictCompatibility) || err != nil { - switch val { - case "SERIALIZABLE", "READ-UNCOMMITTED": - return ErrUnsupportedValueForVar.GenWithStackByArgs(name, val) + switch val { + case "SERIALIZABLE", "READ-UNCOMMITTED": + skipIsolationLevelCheck, err := GetSessionSystemVar(s, TiDBSkipIsolationLevelCheck) + unSupportedValueForVarErr := ErrUnsupportedValueForVar.GenWithStackByArgs(name, val) + if !TiDBOptOn(skipIsolationLevelCheck) || err != nil { + return unSupportedValueForVarErr } + log.Warn("", zap.Error(unSupportedValueForVarErr)) } s.TxnIsolationLevelOneShot.State = 1 s.TxnIsolationLevelOneShot.Value = val diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index 76e12488be3c2..841f7c0b5ca46 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -673,7 +673,7 @@ var defaultSysVars = []*SysVar{ {ScopeSession, TiDBOptimizerSelectivityLevel, strconv.Itoa(DefTiDBOptimizerSelectivityLevel)}, {ScopeGlobal | ScopeSession, TiDBEnableWindowFunction, BoolToIntStr(DefEnableWindowFunction)}, {ScopeGlobal | ScopeSession, TiDBEnableFastAnalyze, BoolToIntStr(DefTiDBUseFastAnalyze)}, - {ScopeGlobal | ScopeSession, TiDBStrictCompatibility, BoolToIntStr(DefTiDBStrictCompatibility)}, + {ScopeGlobal | ScopeSession, TiDBSkipIsolationLevelCheck, BoolToIntStr(DefTiDBSkipIsolationLevelCheck)}, /* The following variable is defined as session scope but is actually server scope. */ {ScopeSession, TiDBGeneralLog, strconv.Itoa(DefTiDBGeneralLog)}, {ScopeSession, TiDBSlowLogThreshold, strconv.Itoa(logutil.DefaultSlowThreshold)}, diff --git a/sessionctx/variable/tidb_vars.go b/sessionctx/variable/tidb_vars.go index 7b20c2d979a47..5040f66620822 100644 --- a/sessionctx/variable/tidb_vars.go +++ b/sessionctx/variable/tidb_vars.go @@ -132,9 +132,9 @@ const ( // TiDBCheckMb4ValueInUTF8 is used to control whether to enable the check wrong utf8 value. TiDBCheckMb4ValueInUTF8 = "tidb_check_mb4_value_in_utf8" - // tidb_strict_compatibility is used to control whether to return error when set unsupported transaction + // tidb_skip_isolation_level_check is used to control whether to return error when set unsupported transaction // isolation level. - TiDBStrictCompatibility = "tidb_strict_compatibility" + TiDBSkipIsolationLevelCheck = "tidb_skip_isolation_level_check" ) // TiDB system variable names that both in session and global scope. @@ -299,7 +299,7 @@ const ( DefEnableWindowFunction = false DefTiDBDDLSlowOprThreshold = 300 DefTiDBUseFastAnalyze = false - DefTiDBStrictCompatibility = false + DefTiDBSkipIsolationLevelCheck = false ) // Process global variables. diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index 2e7cb0cf4431c..522e357cf1768 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -16,6 +16,8 @@ package variable import ( "encoding/json" "fmt" + "github.com/pingcap/log" + "go.uber.org/zap" "math" "strconv" "strings" @@ -427,13 +429,14 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, if !exists { return "", ErrWrongValueForVar.GenWithStackByArgs(name, value) } - - strictCompatibility, err := GetSessionSystemVar(vars, TiDBStrictCompatibility) - if !TiDBOptOn(strictCompatibility) || err != nil { - switch upVal { - case "SERIALIZABLE", "READ-UNCOMMITTED": - return "", ErrUnsupportedValueForVar.GenWithStackByArgs(name, value) + switch upVal { + case "SERIALIZABLE", "READ-UNCOMMITTED": + skipIsolationLevelCheck, err := GetSessionSystemVar(vars, TiDBSkipIsolationLevelCheck) + unSupportedValueForVarErr := ErrUnsupportedValueForVar.GenWithStackByArgs(name, value) + if !TiDBOptOn(skipIsolationLevelCheck) || err != nil { + return "", unSupportedValueForVarErr } + log.Warn("", zap.Error(unSupportedValueForVarErr)) } return upVal, nil case TiDBInitChunkSize: From 8ddd5c5c065bf81309dfd32147c82ca8e209848b Mon Sep 17 00:00:00 2001 From: marsishandsome Date: Tue, 9 Apr 2019 09:38:39 +0800 Subject: [PATCH 06/12] fix log warning --- sessionctx/variable/session.go | 4 +--- sessionctx/variable/varsutil.go | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 1e69245f81751..10235bbc12b5a 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -17,8 +17,6 @@ import ( "bytes" "crypto/tls" "fmt" - "github.com/pingcap/log" - "go.uber.org/zap" "strconv" "strings" "sync" @@ -610,7 +608,7 @@ func (s *SessionVars) SetSystemVar(name string, val string) error { if !TiDBOptOn(skipIsolationLevelCheck) || err != nil { return unSupportedValueForVarErr } - log.Warn("", zap.Error(unSupportedValueForVarErr)) + s.StmtCtx.AppendWarning(unSupportedValueForVarErr) } s.TxnIsolationLevelOneShot.State = 1 s.TxnIsolationLevelOneShot.Value = val diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index 522e357cf1768..ac860ec77674f 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -16,8 +16,6 @@ package variable import ( "encoding/json" "fmt" - "github.com/pingcap/log" - "go.uber.org/zap" "math" "strconv" "strings" @@ -436,7 +434,7 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, if !TiDBOptOn(skipIsolationLevelCheck) || err != nil { return "", unSupportedValueForVarErr } - log.Warn("", zap.Error(unSupportedValueForVarErr)) + vars.StmtCtx.AppendWarning(unSupportedValueForVarErr) } return upVal, nil case TiDBInitChunkSize: From 4c5eddddff33ea61edf2cd4eed1880621864d06d Mon Sep 17 00:00:00 2001 From: marsishandsome Date: Tue, 9 Apr 2019 10:13:38 +0800 Subject: [PATCH 07/12] fix error msg --- sessionctx/variable/session.go | 9 ++++++--- sessionctx/variable/varsutil.go | 9 ++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 10235bbc12b5a..9673ace21b665 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -604,11 +604,14 @@ func (s *SessionVars) SetSystemVar(name string, val string) error { switch val { case "SERIALIZABLE", "READ-UNCOMMITTED": skipIsolationLevelCheck, err := GetSessionSystemVar(s, TiDBSkipIsolationLevelCheck) - unSupportedValueForVarErr := ErrUnsupportedValueForVar.GenWithStackByArgs(name, val) + returnErr := ErrUnsupportedValueForVar.GenWithStackByArgs(name, val) + if err != nil { + returnErr = err + } if !TiDBOptOn(skipIsolationLevelCheck) || err != nil { - return unSupportedValueForVarErr + return returnErr } - s.StmtCtx.AppendWarning(unSupportedValueForVarErr) + s.StmtCtx.AppendWarning(returnErr) } s.TxnIsolationLevelOneShot.State = 1 s.TxnIsolationLevelOneShot.Value = val diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index ac860ec77674f..5b61d0c3c1211 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -430,11 +430,14 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, switch upVal { case "SERIALIZABLE", "READ-UNCOMMITTED": skipIsolationLevelCheck, err := GetSessionSystemVar(vars, TiDBSkipIsolationLevelCheck) - unSupportedValueForVarErr := ErrUnsupportedValueForVar.GenWithStackByArgs(name, value) + returnErr := ErrUnsupportedValueForVar.GenWithStackByArgs(name, value) + if err != nil { + returnErr = err + } if !TiDBOptOn(skipIsolationLevelCheck) || err != nil { - return "", unSupportedValueForVarErr + return "", returnErr } - vars.StmtCtx.AppendWarning(unSupportedValueForVarErr) + vars.StmtCtx.AppendWarning(returnErr) } return upVal, nil case TiDBInitChunkSize: From 0bda114027397483788e6a8893b226aecd45eb82 Mon Sep 17 00:00:00 2001 From: marsishandsome Date: Tue, 9 Apr 2019 11:10:26 +0800 Subject: [PATCH 08/12] add warning check in ut --- executor/set_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/executor/set_test.go b/executor/set_test.go index 1480a64798865..9620270a895f2 100644 --- a/executor/set_test.go +++ b/executor/set_test.go @@ -311,6 +311,9 @@ func (s *testSuite2) TestSetVar(c *C) { tk.MustExec("SET GLOBAL tidb_skip_isolation_level_check = 1") tk.MustExec("SET SESSION tidb_skip_isolation_level_check = 1") tk.MustExec("SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE") + tk.MustQuery("show warnings").Check(testkit.Rows( + "Warning 1105 variable 'tx_isolation' does not yet support value: SERIALIZABLE", + "Warning 1105 variable 'transaction_isolation' does not yet support value: SERIALIZABLE")) tk.MustQuery("select @@session.tx_isolation").Check(testkit.Rows("SERIALIZABLE")) tk.MustQuery("select @@session.transaction_isolation").Check(testkit.Rows("SERIALIZABLE")) @@ -318,6 +321,9 @@ func (s *testSuite2) TestSetVar(c *C) { tk.MustExec("SET GLOBAL tidb_skip_isolation_level_check = 0") tk.MustExec("SET SESSION tidb_skip_isolation_level_check = 1") tk.MustExec("SET GLOBAL TRANSACTION ISOLATION LEVEL READ UNCOMMITTED") + tk.MustQuery("show warnings").Check(testkit.Rows( + "Warning 1105 variable 'tx_isolation' does not yet support value: READ-UNCOMMITTED", + "Warning 1105 variable 'transaction_isolation' does not yet support value: READ-UNCOMMITTED")) tk.MustQuery("select @@global.tx_isolation").Check(testkit.Rows("READ-UNCOMMITTED")) tk.MustQuery("select @@global.transaction_isolation").Check(testkit.Rows("READ-UNCOMMITTED")) } From a388359063818c7916f8c56bb1261b80c2332a2b Mon Sep 17 00:00:00 2001 From: marsishandsome Date: Tue, 9 Apr 2019 11:24:28 +0800 Subject: [PATCH 09/12] fix ut --- executor/set_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/executor/set_test.go b/executor/set_test.go index 9620270a895f2..efc76ed7cace1 100644 --- a/executor/set_test.go +++ b/executor/set_test.go @@ -326,6 +326,10 @@ func (s *testSuite2) TestSetVar(c *C) { "Warning 1105 variable 'transaction_isolation' does not yet support value: READ-UNCOMMITTED")) tk.MustQuery("select @@global.tx_isolation").Check(testkit.Rows("READ-UNCOMMITTED")) tk.MustQuery("select @@global.transaction_isolation").Check(testkit.Rows("READ-UNCOMMITTED")) + + // test skip isolation level check: reset + tk.MustExec("SET GLOBAL tidb_skip_isolation_level_check = 0") + tk.MustExec("SET SESSION tidb_skip_isolation_level_check = 0") } func (s *testSuite2) TestSetCharset(c *C) { @@ -625,6 +629,8 @@ func (s *testSuite2) TestValidateSetVar(c *C) { result = tk.MustQuery("select @@tx_isolation;") result.Check(testkit.Rows("REPEATABLE-READ")) + tk.MustExec("SET GLOBAL tidb_skip_isolation_level_check = 0") + tk.MustExec("SET SESSION tidb_skip_isolation_level_check = 0") _, err = tk.Exec("set @@tx_isolation='SERIALIZABLE'") c.Assert(terror.ErrorEqual(err, variable.ErrUnsupportedValueForVar), IsTrue, Commentf("err %v", err)) } From 18957ec633672ebc8141fbe6c1491318eff2a427 Mon Sep 17 00:00:00 2001 From: marsishandsome Date: Tue, 9 Apr 2019 11:36:43 +0800 Subject: [PATCH 10/12] fix ut --- executor/set_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/executor/set_test.go b/executor/set_test.go index efc76ed7cace1..1633c805b09f5 100644 --- a/executor/set_test.go +++ b/executor/set_test.go @@ -313,7 +313,7 @@ func (s *testSuite2) TestSetVar(c *C) { tk.MustExec("SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE") tk.MustQuery("show warnings").Check(testkit.Rows( "Warning 1105 variable 'tx_isolation' does not yet support value: SERIALIZABLE", - "Warning 1105 variable 'transaction_isolation' does not yet support value: SERIALIZABLE")) + "Warning 1105 variable 'transaction_isolation' does not yet support value: SERIALIZABLE")) tk.MustQuery("select @@session.tx_isolation").Check(testkit.Rows("SERIALIZABLE")) tk.MustQuery("select @@session.transaction_isolation").Check(testkit.Rows("SERIALIZABLE")) @@ -323,7 +323,7 @@ func (s *testSuite2) TestSetVar(c *C) { tk.MustExec("SET GLOBAL TRANSACTION ISOLATION LEVEL READ UNCOMMITTED") tk.MustQuery("show warnings").Check(testkit.Rows( "Warning 1105 variable 'tx_isolation' does not yet support value: READ-UNCOMMITTED", - "Warning 1105 variable 'transaction_isolation' does not yet support value: READ-UNCOMMITTED")) + "Warning 1105 variable 'transaction_isolation' does not yet support value: READ-UNCOMMITTED")) tk.MustQuery("select @@global.tx_isolation").Check(testkit.Rows("READ-UNCOMMITTED")) tk.MustQuery("select @@global.transaction_isolation").Check(testkit.Rows("READ-UNCOMMITTED")) From 470b0dd62b16ae5fc17e97ee057b6e6c756143cb Mon Sep 17 00:00:00 2001 From: marsishandsome Date: Thu, 11 Apr 2019 10:01:44 +0800 Subject: [PATCH 11/12] change error message --- executor/set_test.go | 12 ++++++------ sessionctx/variable/session.go | 2 +- sessionctx/variable/sysvar.go | 1 + sessionctx/variable/varsutil.go | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/executor/set_test.go b/executor/set_test.go index 1633c805b09f5..523247bc08dfa 100644 --- a/executor/set_test.go +++ b/executor/set_test.go @@ -298,12 +298,12 @@ func (s *testSuite2) TestSetVar(c *C) { // test skip isolation level check: error _, err = tk.Exec("SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE") - c.Assert(terror.ErrorEqual(err, variable.ErrUnsupportedValueForVar), IsTrue, Commentf("err %v", err)) + c.Assert(terror.ErrorEqual(err, variable.ErrUnsupportedIsolationLevel), IsTrue, Commentf("err %v", err)) tk.MustQuery("select @@session.tx_isolation").Check(testkit.Rows("READ-COMMITTED")) tk.MustQuery("select @@session.transaction_isolation").Check(testkit.Rows("READ-COMMITTED")) _, err = tk.Exec("SET GLOBAL TRANSACTION ISOLATION LEVEL SERIALIZABLE") - c.Assert(terror.ErrorEqual(err, variable.ErrUnsupportedValueForVar), IsTrue, Commentf("err %v", err)) + c.Assert(terror.ErrorEqual(err, variable.ErrUnsupportedIsolationLevel), IsTrue, Commentf("err %v", err)) tk.MustQuery("select @@global.tx_isolation").Check(testkit.Rows("READ-COMMITTED")) tk.MustQuery("select @@global.transaction_isolation").Check(testkit.Rows("READ-COMMITTED")) @@ -312,8 +312,8 @@ func (s *testSuite2) TestSetVar(c *C) { tk.MustExec("SET SESSION tidb_skip_isolation_level_check = 1") tk.MustExec("SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE") tk.MustQuery("show warnings").Check(testkit.Rows( - "Warning 1105 variable 'tx_isolation' does not yet support value: SERIALIZABLE", - "Warning 1105 variable 'transaction_isolation' does not yet support value: SERIALIZABLE")) + "Warning 1105 The isolation level 'SERIALIZABLE' is not supported. Set tidb_skip_isolation_level_check=1 to skip this error", + "Warning 1105 The isolation level 'SERIALIZABLE' is not supported. Set tidb_skip_isolation_level_check=1 to skip this error")) tk.MustQuery("select @@session.tx_isolation").Check(testkit.Rows("SERIALIZABLE")) tk.MustQuery("select @@session.transaction_isolation").Check(testkit.Rows("SERIALIZABLE")) @@ -322,8 +322,8 @@ func (s *testSuite2) TestSetVar(c *C) { tk.MustExec("SET SESSION tidb_skip_isolation_level_check = 1") tk.MustExec("SET GLOBAL TRANSACTION ISOLATION LEVEL READ UNCOMMITTED") tk.MustQuery("show warnings").Check(testkit.Rows( - "Warning 1105 variable 'tx_isolation' does not yet support value: READ-UNCOMMITTED", - "Warning 1105 variable 'transaction_isolation' does not yet support value: READ-UNCOMMITTED")) + "Warning 1105 The isolation level 'READ-UNCOMMITTED' is not supported. Set tidb_skip_isolation_level_check=1 to skip this error", + "Warning 1105 The isolation level 'READ-UNCOMMITTED' is not supported. Set tidb_skip_isolation_level_check=1 to skip this error")) tk.MustQuery("select @@global.tx_isolation").Check(testkit.Rows("READ-UNCOMMITTED")) tk.MustQuery("select @@global.transaction_isolation").Check(testkit.Rows("READ-UNCOMMITTED")) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 9673ace21b665..15b45dbb5ff11 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -604,7 +604,7 @@ func (s *SessionVars) SetSystemVar(name string, val string) error { switch val { case "SERIALIZABLE", "READ-UNCOMMITTED": skipIsolationLevelCheck, err := GetSessionSystemVar(s, TiDBSkipIsolationLevelCheck) - returnErr := ErrUnsupportedValueForVar.GenWithStackByArgs(name, val) + returnErr := ErrUnsupportedIsolationLevel.GenWithStackByArgs(val) if err != nil { returnErr = err } diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index 841f7c0b5ca46..5d705b8cf50e8 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -85,6 +85,7 @@ var ( ErrTruncatedWrongValue = terror.ClassVariable.New(CodeTruncatedWrongValue, mysql.MySQLErrName[mysql.ErrTruncatedWrongValue]) ErrMaxPreparedStmtCountReached = terror.ClassVariable.New(CodeMaxPreparedStmtCountReached, mysql.MySQLErrName[mysql.ErrMaxPreparedStmtCountReached]) ErrUnsupportedValueForVar = terror.ClassVariable.New(CodeUnknownStatusVar, "variable '%s' does not yet support value: %s") + ErrUnsupportedIsolationLevel = terror.ClassVariable.New(CodeUnknownStatusVar, "The isolation level '%s' is not supported. Set tidb_skip_isolation_level_check=1 to skip this error") ) func init() { diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index 5b61d0c3c1211..e58b245321ad9 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -430,7 +430,7 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, switch upVal { case "SERIALIZABLE", "READ-UNCOMMITTED": skipIsolationLevelCheck, err := GetSessionSystemVar(vars, TiDBSkipIsolationLevelCheck) - returnErr := ErrUnsupportedValueForVar.GenWithStackByArgs(name, value) + returnErr := ErrUnsupportedIsolationLevel.GenWithStackByArgs(value) if err != nil { returnErr = err } From 2c7535692fb0b687a6da422cad2207c5480c3842 Mon Sep 17 00:00:00 2001 From: marsishandsome Date: Thu, 11 Apr 2019 11:45:08 +0800 Subject: [PATCH 12/12] deduplicate two same warnings --- executor/set_test.go | 2 -- sessionctx/variable/session.go | 8 +++++++- sessionctx/variable/varsutil.go | 8 +++++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/executor/set_test.go b/executor/set_test.go index 523247bc08dfa..93af8e0944c7d 100644 --- a/executor/set_test.go +++ b/executor/set_test.go @@ -312,7 +312,6 @@ func (s *testSuite2) TestSetVar(c *C) { tk.MustExec("SET SESSION tidb_skip_isolation_level_check = 1") tk.MustExec("SET SESSION TRANSACTION ISOLATION LEVEL SERIALIZABLE") tk.MustQuery("show warnings").Check(testkit.Rows( - "Warning 1105 The isolation level 'SERIALIZABLE' is not supported. Set tidb_skip_isolation_level_check=1 to skip this error", "Warning 1105 The isolation level 'SERIALIZABLE' is not supported. Set tidb_skip_isolation_level_check=1 to skip this error")) tk.MustQuery("select @@session.tx_isolation").Check(testkit.Rows("SERIALIZABLE")) tk.MustQuery("select @@session.transaction_isolation").Check(testkit.Rows("SERIALIZABLE")) @@ -322,7 +321,6 @@ func (s *testSuite2) TestSetVar(c *C) { tk.MustExec("SET SESSION tidb_skip_isolation_level_check = 1") tk.MustExec("SET GLOBAL TRANSACTION ISOLATION LEVEL READ UNCOMMITTED") tk.MustQuery("show warnings").Check(testkit.Rows( - "Warning 1105 The isolation level 'READ-UNCOMMITTED' is not supported. Set tidb_skip_isolation_level_check=1 to skip this error", "Warning 1105 The isolation level 'READ-UNCOMMITTED' is not supported. Set tidb_skip_isolation_level_check=1 to skip this error")) tk.MustQuery("select @@global.tx_isolation").Check(testkit.Rows("READ-UNCOMMITTED")) tk.MustQuery("select @@global.transaction_isolation").Check(testkit.Rows("READ-UNCOMMITTED")) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 15b45dbb5ff11..220e1adf008bb 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -611,7 +611,13 @@ func (s *SessionVars) SetSystemVar(name string, val string) error { if !TiDBOptOn(skipIsolationLevelCheck) || err != nil { return returnErr } - s.StmtCtx.AppendWarning(returnErr) + //SET TRANSACTION ISOLATION LEVEL will affect two internal variables: + // 1. tx_isolation + // 2. transaction_isolation + // The following if condition is used to deduplicate two same warnings. + if name == "transaction_isolation" { + s.StmtCtx.AppendWarning(returnErr) + } } s.TxnIsolationLevelOneShot.State = 1 s.TxnIsolationLevelOneShot.Value = val diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index e58b245321ad9..c744ed4fd92a0 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -437,7 +437,13 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, if !TiDBOptOn(skipIsolationLevelCheck) || err != nil { return "", returnErr } - vars.StmtCtx.AppendWarning(returnErr) + //SET TRANSACTION ISOLATION LEVEL will affect two internal variables: + // 1. tx_isolation + // 2. transaction_isolation + // The following if condition is used to deduplicate two same warnings. + if name == "transaction_isolation" { + vars.StmtCtx.AppendWarning(returnErr) + } } return upVal, nil case TiDBInitChunkSize: