From 2d33a19372030a506904bf4cfe52cbe55e1f9e66 Mon Sep 17 00:00:00 2001 From: Shuaipeng Yu Date: Wed, 18 Mar 2020 21:33:01 +0800 Subject: [PATCH 1/2] *: read committed transaction gets a wrong snapshot with subquery Signed-off-by: Shuaipeng Yu --- executor/builder.go | 4 +++- session/pessimistic_test.go | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/executor/builder.go b/executor/builder.go index 0cd72a884f580..d5a01ca9b0602 100644 --- a/executor/builder.go +++ b/executor/builder.go @@ -1679,6 +1679,9 @@ func (b *executorBuilder) updateForUpdateTSIfNeeded(selectPlan plannercore.Physi // refreshForUpdateTSForRC is used to refresh the for-update-ts for reading data at read consistency level in pessimistic transaction. // It could use the cached tso from the statement future to avoid get tso many times. func (b *executorBuilder) refreshForUpdateTSForRC() error { + defer func() { + b.snapshotTS = b.ctx.GetSessionVars().TxnCtx.GetForUpdateTS() + }() future := b.ctx.GetSessionVars().TxnCtx.GetStmtFutureForRC() if future == nil { return nil @@ -1694,7 +1697,6 @@ func (b *executorBuilder) refreshForUpdateTSForRC() error { if err := UpdateForUpdateTS(b.ctx, newForUpdateTS); err != nil { return err } - b.snapshotTS = b.ctx.GetSessionVars().TxnCtx.GetForUpdateTS() return nil } diff --git a/session/pessimistic_test.go b/session/pessimistic_test.go index 46280d48b99fa..1ca9fb83e2fef 100644 --- a/session/pessimistic_test.go +++ b/session/pessimistic_test.go @@ -1144,3 +1144,21 @@ func (s *testPessimisticSuite) TestRollbackWakeupBlockedTxn(c *C) { c.Assert(failpoint.Disable("github.com/pingcap/tidb/store/tikv/txnExpireRetTTL"), IsNil) c.Assert(failpoint.Disable("github.com/pingcap/tidb/store/tikv/getTxnStatusDelay"), IsNil) } + +func (s *testPessimisticSuite) TestRCSubQuery(c *C) { + tk := testkit.NewTestKitWithInit(c, s.store) + tk.MustExec("drop table if exists t, t1") + tk.MustExec("create table `t` ( `c1` int(11) not null, `c2` int(11) default null, primary key (`c1`) )") + tk.MustExec("insert into t values(1, 3)") + tk.MustExec("create table `t1` ( `c1` int(11) not null, `c2` int(11) default null, primary key (`c1`) )") + tk.MustExec("insert into t1 values(1, 3)") + tk.MustExec("set transaction isolation level read committed") + tk.MustExec("begin pessimistic") + + tk2 := testkit.NewTestKitWithInit(c, s.store) + tk2.MustExec("update t1 set c2 = c2 + 1") + + tk.MustQuery("select * from t1 where c1 = (select 1) and 1=1;").Check(testkit.Rows("1 4")) + tk.MustQuery("select * from t1 where c1 = (select c1 from t where c1 = 1) and 1=1;").Check(testkit.Rows("1 4")) + tk.MustExec("rollback") +} From c4f7d02dbb91a9cd5e5a063a1a5fe0d06f5ebf81 Mon Sep 17 00:00:00 2001 From: Shuaipeng Yu Date: Wed, 18 Mar 2020 22:11:39 +0800 Subject: [PATCH 2/2] fix check Signed-off-by: Shuaipeng Yu --- executor/builder.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/executor/builder.go b/executor/builder.go index d5a01ca9b0602..740e2ad8070d6 100644 --- a/executor/builder.go +++ b/executor/builder.go @@ -1694,10 +1694,7 @@ func (b *executorBuilder) refreshForUpdateTSForRC() error { } b.ctx.GetSessionVars().TxnCtx.SetStmtFutureForRC(nil) // If newForUpdateTS is 0, it will force to get a new for-update-ts from PD. - if err := UpdateForUpdateTS(b.ctx, newForUpdateTS); err != nil { - return err - } - return nil + return UpdateForUpdateTS(b.ctx, newForUpdateTS) } func (b *executorBuilder) buildAnalyzeIndexPushdown(task plannercore.AnalyzeIndexTask, opts map[ast.AnalyzeOptionType]uint64, autoAnalyze string) *analyzeTask {