From 0acf94eeba6ea87c00a09a8387eb593dd17a6ff7 Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Mon, 12 Aug 2019 14:01:53 +0800 Subject: [PATCH 1/2] planner: "for update" should not work in a single autocommit statement --- planner/core/point_get_plan.go | 13 ++++++-- planner/core/point_get_plan_test.go | 51 ++++++++++++++++++++++++++--- 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/planner/core/point_get_plan.go b/planner/core/point_get_plan.go index 5d48f282fbc60..b0b17e6f84e59 100644 --- a/planner/core/point_get_plan.go +++ b/planner/core/point_get_plan.go @@ -95,6 +95,9 @@ func (p *PointGetPlan) ExplainInfo() string { fmt.Fprintf(buffer, ", handle:%d", p.Handle) } } + if p.Lock { + fmt.Fprintf(buffer, ", lock") + } return buffer.String() } @@ -148,8 +151,14 @@ func TryFastPlan(ctx sessionctx.Context, node ast.Node) Plan { return tableDual.Init(ctx, &property.StatsInfo{}) } if x.LockTp == ast.SelectLockForUpdate { - fp.Lock = true - fp.IsForUpdate = true + // Locking of rows for update using SELECT FOR UPDATE only applies when autocommit + // is disabled (either by beginning transaction with START TRANSACTION or by setting + // autocommit to 0. If autocommit is enabled, the rows matching the specification are not locked. + // See https://dev.mysql.com/doc/refman/5.7/en/innodb-locking-reads.html + if ctx.GetSessionVars().InTxn() { + fp.Lock = true + fp.IsForUpdate = true + } } return fp } diff --git a/planner/core/point_get_plan_test.go b/planner/core/point_get_plan_test.go index eac7937801fbc..3ff3cf80f328c 100644 --- a/planner/core/point_get_plan_test.go +++ b/planner/core/point_get_plan_test.go @@ -14,9 +14,13 @@ package core_test import ( + "fmt" "math" + "strings" . "github.com/pingcap/check" + "github.com/pingcap/tidb/domain" + "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/metrics" "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/util/testkit" @@ -27,20 +31,31 @@ import ( var _ = Suite(&testPointGetSuite{}) type testPointGetSuite struct { + store kv.Storage + dom *domain.Domain } -func (s *testPointGetSuite) TestPointGetPlanCache(c *C) { - defer testleak.AfterTest(c)() +func (s *testPointGetSuite) SetUpSuite(c *C) { + testleak.BeforeTest() store, dom, err := newStoreWithBootstrap() c.Assert(err, IsNil) - tk := testkit.NewTestKit(c, store) + s.store = store + s.dom = dom +} + +func (s *testPointGetSuite) TearDownSuite(c *C) { + s.dom.Close() + s.store.Close() + testleak.AfterTest(c)() +} + +func (s *testPointGetSuite) TestPointGetPlanCache(c *C) { + tk := testkit.NewTestKit(c, s.store) orgEnable := core.PreparedPlanCacheEnabled() orgCapacity := core.PreparedPlanCacheCapacity orgMemGuardRatio := core.PreparedPlanCacheMemoryGuardRatio orgMaxMemory := core.PreparedPlanCacheMaxMemory defer func() { - dom.Close() - store.Close() core.SetPreparedPlanCache(orgEnable) core.PreparedPlanCacheCapacity = orgCapacity core.PreparedPlanCacheMemoryGuardRatio = orgMemGuardRatio @@ -152,3 +167,29 @@ func (s *testPointGetSuite) TestPointGetPlanCache(c *C) { hit = pb.GetCounter().GetValue() c.Check(hit, Equals, float64(2)) } + +func (s *testPointGetSuite) TestPointGetForUpdate(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("create table fu (id int primary key)") + tk.MustExec("insert into fu values (6)") + tk.MustQuery("select * from fu where id = 6 for update").Check(testkit.Rows("6")) + + // In autocommit mode, outside a transaction, "for update" doesn't take effect. + res := tk.MustQuery("explain select * from fu where id = 6 for update") + // Point_Get_1 1.00 root table:fu, handle:6 + opInfo := res.Rows()[0][3] + selectLock := strings.Contains(fmt.Sprintf("%s", opInfo), "lock") + c.Assert(selectLock, IsFalse) + + tk.MustExec("begin") + tk.MustQuery("select * from fu where id = 6 for update").Check(testkit.Rows("6")) + res = tk.MustQuery("explain select * from fu where id = 6 for update") + // Point_Get_1 1.00 root table:fu, handle:6 + opInfo = res.Rows()[0][3] + selectLock = strings.Contains(fmt.Sprintf("%s", opInfo), "lock") + c.Assert(selectLock, IsTrue) + + tk.MustExec("rollback") + +} From 6ceeefb018987034dcb50b61c791b4afe99b91d7 Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Mon, 12 Aug 2019 14:48:30 +0800 Subject: [PATCH 2/2] fix explain test --- cmd/explaintest/r/explain_easy.result | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/explaintest/r/explain_easy.result b/cmd/explaintest/r/explain_easy.result index d06d685d8995f..e0967097003c7 100644 --- a/cmd/explaintest/r/explain_easy.result +++ b/cmd/explaintest/r/explain_easy.result @@ -458,7 +458,7 @@ Projection_3 10000.00 root or(NULL, gt(test.t.a, 1)) └─TableScan_4 10000.00 cop table:t, range:[-inf,+inf], keep order:false, stats:pseudo explain select * from t where a = 1 for update; id count task operator info -Point_Get_1 1.00 root table:t, handle:1 +Point_Get_1 1.00 root table:t, handle:1, lock drop table if exists ta, tb; create table ta (a varchar(20)); create table tb (a varchar(20));