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

planner: "for update" should not work in a single autocommit statement #11715

Merged
merged 3 commits into from
Aug 13, 2019
Merged
Show file tree
Hide file tree
Changes from 2 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: 1 addition & 1 deletion cmd/explaintest/r/explain_easy.result
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
13 changes: 11 additions & 2 deletions planner/core/point_get_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ func (p *PointGetPlan) ExplainInfo() string {
fmt.Fprintf(buffer, ", handle:%d", p.Handle)
}
}
if p.Lock {
fmt.Fprintf(buffer, ", lock")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you print more information about this lock?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example?

}
return buffer.String()
}

Expand Down Expand Up @@ -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
}
Expand Down
51 changes: 46 additions & 5 deletions planner/core/point_get_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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")

}