From 38fa026f793daf0ee4db928bc46e0f15ebe15d4d Mon Sep 17 00:00:00 2001 From: cfzjywxk Date: Fri, 28 Jun 2019 22:10:00 +0800 Subject: [PATCH 1/4] issue#4100 add new variable to default disable usage of get_lock and release_lock functions --- executor/set_test.go | 43 ++++++++++++++++++++++++++++++++ expression/errors.go | 1 + expression/function_traits.go | 8 ++++++ expression/integration_test.go | 1 + expression/scalar_function.go | 5 ++++ expression/typeinfer_test.go | 1 + session/session.go | 1 + sessionctx/variable/session.go | 6 +++++ sessionctx/variable/sysvar.go | 1 + sessionctx/variable/tidb_vars.go | 4 +++ sessionctx/variable/varsutil.go | 2 +- 11 files changed, 72 insertions(+), 1 deletion(-) diff --git a/executor/set_test.go b/executor/set_test.go index 022b3149c6ea6..7908fae7f24fb 100644 --- a/executor/set_test.go +++ b/executor/set_test.go @@ -20,6 +20,7 @@ import ( . "github.com/pingcap/check" "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/config" + "github.com/pingcap/tidb/expression" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/util/testkit" @@ -741,3 +742,45 @@ func (s *testSuite2) TestSelectGlobalVar(c *C) { err = tk.ExecToErr("select @@global.invalid") c.Assert(terror.ErrorEqual(err, variable.UnknownSystemVar), IsTrue, Commentf("err %v", err)) } + +func (s *testSuite2) TestEnableNoopFunctionsVar(c *C) { + tk := testkit.NewTestKit(c, s.store) + + // test for tidb_enable_noop_functions + tk.MustQuery(`select @@global.tidb_enable_noop_functions;`).Check(testkit.Rows("0")) + tk.MustQuery(`select @@tidb_enable_noop_functions;`).Check(testkit.Rows("0")) + + _, err := tk.Exec(`select get_lock('lock1', 2);`) + c.Assert(terror.ErrorEqual(err, expression.ErrFunctionsNoopImpl), IsTrue, Commentf("err %v", err)) + _, err = tk.Exec(`select release_lock('lock1');`) + c.Assert(terror.ErrorEqual(err, expression.ErrFunctionsNoopImpl), IsTrue, Commentf("err %v", err)) + + // change session var to 1 + tk.MustExec(`set tidb_enable_noop_functions=1;`) + tk.MustQuery(`select @@tidb_enable_noop_functions;`).Check(testkit.Rows("1")) + tk.MustQuery(`select @@global.tidb_enable_noop_functions;`).Check(testkit.Rows("0")) + tk.MustQuery(`select get_lock("lock", 10)`).Check(testkit.Rows("1")) + tk.MustQuery(`select release_lock("lock")`).Check(testkit.Rows("1")) + + // restore to 0 + tk.MustExec(`set tidb_enable_noop_functions=0;`) + tk.MustQuery(`select @@tidb_enable_noop_functions;`).Check(testkit.Rows("0")) + tk.MustQuery(`select @@global.tidb_enable_noop_functions;`).Check(testkit.Rows("0")) + + _, err = tk.Exec(`select get_lock('lock2', 10);`) + c.Assert(terror.ErrorEqual(err, expression.ErrFunctionsNoopImpl), IsTrue, Commentf("err %v", err)) + _, err = tk.Exec(`select release_lock('lock2');`) + c.Assert(terror.ErrorEqual(err, expression.ErrFunctionsNoopImpl), IsTrue, Commentf("err %v", err)) + + // set test + _, err = tk.Exec(`set tidb_enable_noop_functions='abc'`) + c.Assert(err, NotNil) + _, err = tk.Exec(`set tidb_enable_noop_functions=11`) + c.Assert(err, NotNil) + tk.MustExec(`set tidb_enable_noop_functions="off";`) + tk.MustQuery(`select @@tidb_enable_noop_functions;`).Check(testkit.Rows("off")) + tk.MustExec(`set tidb_enable_noop_functions="on";`) + tk.MustQuery(`select @@tidb_enable_noop_functions;`).Check(testkit.Rows("on")) + tk.MustExec(`set tidb_enable_noop_functions=0;`) + tk.MustQuery(`select @@tidb_enable_noop_functions;`).Check(testkit.Rows("0")) +} diff --git a/expression/errors.go b/expression/errors.go index 1405300dc07ab..1e7744d5fcbb7 100644 --- a/expression/errors.go +++ b/expression/errors.go @@ -28,6 +28,7 @@ var ( ErrRegexp = terror.ClassExpression.New(mysql.ErrRegexp, mysql.MySQLErrName[mysql.ErrRegexp]) ErrOperandColumns = terror.ClassExpression.New(mysql.ErrOperandColumns, mysql.MySQLErrName[mysql.ErrOperandColumns]) ErrCutValueGroupConcat = terror.ClassExpression.New(mysql.ErrCutValueGroupConcat, mysql.MySQLErrName[mysql.ErrCutValueGroupConcat]) + ErrFunctionsNoopImpl = terror.ClassExpression.New(mysql.ErrNotSupportedYet, "function %s has only noop implementation in tidb now, use tidb_enable_noop_functions to enable these functions") // All the un-exported errors are defined here: errFunctionNotExists = terror.ClassExpression.New(mysql.ErrSpDoesNotExist, mysql.MySQLErrName[mysql.ErrSpDoesNotExist]) diff --git a/expression/function_traits.go b/expression/function_traits.go index f0c1e88c61451..b25ae90869907 100644 --- a/expression/function_traits.go +++ b/expression/function_traits.go @@ -156,3 +156,11 @@ var mutableEffectsFunctions = map[string]struct{}{ ast.GetVar: {}, ast.AnyValue: {}, } + +// some functions like "get_lock" and "release_lock" currently do NOT have +// right implementations, but may have noop ones(like with any inputs, always return 1) +// if apps really need these "funcs" to run, we offer sys var(tidb_enable_noop_functions) to enable noop usage +var noopFuncs = map[string]struct{}{ + ast.GetLock: {}, + ast.ReleaseLock: {}, +} diff --git a/expression/integration_test.go b/expression/integration_test.go index c34f545dcb976..268787735686b 100644 --- a/expression/integration_test.go +++ b/expression/integration_test.go @@ -202,6 +202,7 @@ func (s *testIntegrationSuite) TestMiscellaneousBuiltin(c *C) { tk.MustQuery("select a,any_value(b),sum(c) from t1 group by a order by a;").Check(testkit.Rows("1 10 0", "2 30 0")) // for locks + tk.MustExec(`set tidb_enable_noop_functions=1;`) result := tk.MustQuery(`SELECT GET_LOCK('test_lock1', 10);`) result.Check(testkit.Rows("1")) result = tk.MustQuery(`SELECT GET_LOCK('test_lock2', 10);`) diff --git a/expression/scalar_function.go b/expression/scalar_function.go index 77ce863a39406..200360323df44 100644 --- a/expression/scalar_function.go +++ b/expression/scalar_function.go @@ -82,6 +82,11 @@ func newFunctionImpl(ctx sessionctx.Context, fold bool, funcName string, retType if !ok { return nil, errFunctionNotExists.GenWithStackByArgs("FUNCTION", funcName) } + if !ctx.GetSessionVars().EnableNoopFuncs { + if _, ok := noopFuncs[funcName]; ok { + return nil, ErrFunctionsNoopImpl.GenWithStackByArgs(funcName) + } + } funcArgs := make([]Expression, len(args)) copy(funcArgs, args) f, err := fc.getFunction(ctx, funcArgs) diff --git a/expression/typeinfer_test.go b/expression/typeinfer_test.go index 9c0f2fe00472c..9ccaaffdb11ce 100644 --- a/expression/typeinfer_test.go +++ b/expression/typeinfer_test.go @@ -103,6 +103,7 @@ func (s *testInferTypeSuite) TestInferType(c *C) { c_year year )` testKit.MustExec(sql) + testKit.MustExec(`set tidb_enable_noop_functions=1;`) var tests []typeInferTestCase tests = append(tests, s.createTestCase4Constants()...) diff --git a/session/session.go b/session/session.go index 00964da02a8d0..8eb487c731f15 100644 --- a/session/session.go +++ b/session/session.go @@ -1745,6 +1745,7 @@ var builtinGlobalVariable = []string{ variable.TiDBEnableWindowFunction, variable.TiDBEnableFastAnalyze, variable.TiDBExpensiveQueryTimeThreshold, + variable.TiDBEnableNoopFuncs, } var ( diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index fcc1a03c72a3f..f3c99367e51ef 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -392,6 +392,9 @@ type SessionVars struct { // ConnectionInfo indicates current connection info used by current session, only be lazy assigned by plugin. ConnectionInfo *ConnectionInfo + + // use noop funcs or not + EnableNoopFuncs bool } // ConnectionInfo present connection used by audit. @@ -444,6 +447,7 @@ func NewSessionVars() *SessionVars { SlowQueryFile: config.GetGlobalConfig().Log.SlowQueryFile, WaitSplitRegionFinish: DefTiDBWaitSplitRegionFinish, WaitSplitRegionTimeout: DefWaitSplitRegionTimeout, + EnableNoopFuncs: DefTiDBEnableNoopFuncs, } vars.Concurrency = Concurrency{ IndexLookupConcurrency: DefIndexLookupConcurrency, @@ -818,6 +822,8 @@ func (s *SessionVars) SetSystemVar(name string, val string) error { } case TiDBLowResolutionTSO: s.LowResolutionTSO = TiDBOptOn(val) + case TiDBEnableNoopFuncs: + s.EnableNoopFuncs = TiDBOptOn(val) } s.systems[name] = val return nil diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index d4de35eb97ebc..fae7664e05030 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -701,6 +701,7 @@ var defaultSysVars = []*SysVar{ {ScopeSession, TiDBWaitSplitRegionTimeout, strconv.Itoa(DefWaitSplitRegionTimeout)}, {ScopeSession, TiDBLowResolutionTSO, "0"}, {ScopeSession, TiDBExpensiveQueryTimeThreshold, strconv.Itoa(DefTiDBExpensiveQueryTimeThreshold)}, + {ScopeGlobal | ScopeSession, TiDBEnableNoopFuncs, BoolToIntStr(DefTiDBEnableNoopFuncs)}, } // SynonymsSysVariables is synonyms of system variables. diff --git a/sessionctx/variable/tidb_vars.go b/sessionctx/variable/tidb_vars.go index fe6c3a7a2fe57..156f7cf008d41 100644 --- a/sessionctx/variable/tidb_vars.go +++ b/sessionctx/variable/tidb_vars.go @@ -277,6 +277,9 @@ const ( // TiDBExpensiveQueryTimeThreshold indicates the time threshold of expensive query. TiDBExpensiveQueryTimeThreshold = "tidb_expensive_query_time_threshold" + + // TiDBEnableNoopFuncs set true will enable using fake funcs(like get_lock release_lock) + TiDBEnableNoopFuncs = "tidb_enable_noop_functions" ) // Default TiDB system variable values. @@ -339,6 +342,7 @@ const ( DefTiDBExpensiveQueryTimeThreshold = 60 // 60s DefTiDBWaitSplitRegionFinish = true DefWaitSplitRegionTimeout = 300 // 300s + DefTiDBEnableNoopFuncs = false ) // Process global variables. diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index a6f833d1d6d8f..92489245bfd76 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -419,7 +419,7 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string, TiDBOptInSubqToJoinAndAgg, TiDBEnableFastAnalyze, TiDBBatchInsert, TiDBDisableTxnAutoRetry, TiDBEnableStreaming, TiDBBatchDelete, TiDBBatchCommit, TiDBEnableCascadesPlanner, TiDBEnableWindowFunction, - TiDBCheckMb4ValueInUTF8, TiDBLowResolutionTSO: + TiDBCheckMb4ValueInUTF8, TiDBLowResolutionTSO, TiDBEnableNoopFuncs: if strings.EqualFold(value, "ON") || value == "1" || strings.EqualFold(value, "OFF") || value == "0" { return value, nil } From e1585db6698752a0e7962929103bc8c95d6e1232 Mon Sep 17 00:00:00 2001 From: cfzjywxk Date: Mon, 8 Jul 2019 11:12:49 +0800 Subject: [PATCH 2/4] merge confilct format --- sessionctx/variable/session.go | 2 +- sessionctx/variable/tidb_vars.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index ff129b9b5e96f..0358209ebc942 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -451,7 +451,7 @@ func NewSessionVars() *SessionVars { WaitSplitRegionFinish: DefTiDBWaitSplitRegionFinish, WaitSplitRegionTimeout: DefWaitSplitRegionTimeout, EnableIndexMerge: false, - EnableNoopFuncs: DefTiDBEnableNoopFuncs, + EnableNoopFuncs: DefTiDBEnableNoopFuncs, } vars.Concurrency = Concurrency{ IndexLookupConcurrency: DefIndexLookupConcurrency, diff --git a/sessionctx/variable/tidb_vars.go b/sessionctx/variable/tidb_vars.go index dc7a4b838a748..fe9a92503fc9c 100644 --- a/sessionctx/variable/tidb_vars.go +++ b/sessionctx/variable/tidb_vars.go @@ -278,8 +278,9 @@ const ( // TiDBExpensiveQueryTimeThreshold indicates the time threshold of expensive query. TiDBExpensiveQueryTimeThreshold = "tidb_expensive_query_time_threshold" - // TiDBEnableIndexMerge indicates to generate IndexMergePath. + // TiDBEnableIndexMerge indicates to generate IndexMergePath. TiDBEnableIndexMerge = "tidb_enable_index_merge" + // TiDBEnableNoopFuncs set true will enable using fake funcs(like get_lock release_lock) TiDBEnableNoopFuncs = "tidb_enable_noop_functions" ) From 1e4809f5168283871770d0bcb9ab2f4f454d09e1 Mon Sep 17 00:00:00 2001 From: cfzjywxk Date: Mon, 8 Jul 2019 11:19:54 +0800 Subject: [PATCH 3/4] merge conficts format --- sessionctx/variable/session.go | 4 ++-- sessionctx/variable/tidb_vars.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 0358209ebc942..8bc0914a5dc8a 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -451,7 +451,7 @@ func NewSessionVars() *SessionVars { WaitSplitRegionFinish: DefTiDBWaitSplitRegionFinish, WaitSplitRegionTimeout: DefWaitSplitRegionTimeout, EnableIndexMerge: false, - EnableNoopFuncs: DefTiDBEnableNoopFuncs, + EnableNoopFuncs: DefTiDBEnableNoopFuncs, } vars.Concurrency = Concurrency{ IndexLookupConcurrency: DefIndexLookupConcurrency, @@ -826,7 +826,7 @@ func (s *SessionVars) SetSystemVar(name string, val string) error { } case TiDBLowResolutionTSO: s.LowResolutionTSO = TiDBOptOn(val) - case TiDBEnableIndexMerge: + case TiDBEnableIndexMerge: s.EnableIndexMerge = TiDBOptOn(val) case TiDBEnableNoopFuncs: s.EnableNoopFuncs = TiDBOptOn(val) diff --git a/sessionctx/variable/tidb_vars.go b/sessionctx/variable/tidb_vars.go index fe9a92503fc9c..79ccecdd0c6aa 100644 --- a/sessionctx/variable/tidb_vars.go +++ b/sessionctx/variable/tidb_vars.go @@ -278,7 +278,7 @@ const ( // TiDBExpensiveQueryTimeThreshold indicates the time threshold of expensive query. TiDBExpensiveQueryTimeThreshold = "tidb_expensive_query_time_threshold" - // TiDBEnableIndexMerge indicates to generate IndexMergePath. + // TiDBEnableIndexMerge indicates to generate IndexMergePath. TiDBEnableIndexMerge = "tidb_enable_index_merge" // TiDBEnableNoopFuncs set true will enable using fake funcs(like get_lock release_lock) From e4f20e5750846e726da197e32d94728f1218bfb3 Mon Sep 17 00:00:00 2001 From: cfzjywxk Date: Mon, 8 Jul 2019 13:03:31 +0800 Subject: [PATCH 4/4] make other tests stable --- store/tikv/client_fail_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/store/tikv/client_fail_test.go b/store/tikv/client_fail_test.go index 78f75bf43a9e6..5fc73b6ff1152 100644 --- a/store/tikv/client_fail_test.go +++ b/store/tikv/client_fail_test.go @@ -50,6 +50,7 @@ func (s *testClientSuite) TestPanicInRecvLoop(c *C) { time.Sleep(time.Second) c.Assert(failpoint.Disable("github.com/pingcap/tidb/store/tikv/panicInFailPendingRequests"), IsNil) c.Assert(failpoint.Disable("github.com/pingcap/tidb/store/tikv/gotErrorInRecvLoop"), IsNil) + time.Sleep(time.Second) req := &tikvrpc.Request{ Type: tikvrpc.CmdEmpty,