From 11786665b9375ad3777a732c2c643a26ffbc984c Mon Sep 17 00:00:00 2001 From: crazycs Date: Sun, 12 Jan 2020 21:36:48 +0800 Subject: [PATCH 1/3] executor: add check data visibility for point get --- executor/point_get_test.go | 30 ++++++++++++++++++++++++++++++ store/tikv/snapshot.go | 5 +++++ store/tikv/txn.go | 5 ----- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/executor/point_get_test.go b/executor/point_get_test.go index 7734480b227ba..8634e1dd3cb6c 100644 --- a/executor/point_get_test.go +++ b/executor/point_get_test.go @@ -14,7 +14,9 @@ package executor_test import ( + "context" "fmt" + "time" . "github.com/pingcap/check" "github.com/pingcap/tidb/domain" @@ -445,3 +447,31 @@ func (s *testPointGetSuite) TestPointGetByRowID(c *C) { "Point_Get_1 1.00 root table:t, handle:1")) tk.MustQuery("select * from t where t._tidb_rowid = 1").Check(testkit.Rows("aaa 12")) } + +func (s *testPointGetSuite) TestPointGetCheckVisibility(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + tk.MustExec("create table t (a varchar(10) key, b int)") + tk.MustExec("insert into t values('1',1)") + tk.MustExec("begin") + txn, err := tk.Se.Txn(false) + c.Assert(err, IsNil) + ts := txn.StartTS() + store := tk.Se.GetStore().(tikv.Storage) + // Update gc safe time for check data visibility. + store.UpdateSPCache(ts+1, time.Now()) + // Test point get. + re, err := tk.Exec("select * from t where a='1'") + c.Assert(err, IsNil) + _, err = session.ResultSetToStringSlice(context.Background(), tk.Se, re) + c.Assert(err, NotNil) + c.Assert(tikv.ErrGCTooEarly.Equal(err), IsTrue) + // Test batch point get. + re, err = tk.Exec("select * from t where a in ('1','2') ") + c.Assert(err, IsNil) + _, err = session.ResultSetToStringSlice(context.Background(), tk.Se, re) + c.Assert(err, NotNil) + c.Assert(tikv.ErrGCTooEarly.Equal(err), IsTrue) + +} diff --git a/store/tikv/snapshot.go b/store/tikv/snapshot.go index 1019eaaa580e2..c6603b92d307c 100644 --- a/store/tikv/snapshot.go +++ b/store/tikv/snapshot.go @@ -266,6 +266,11 @@ func (s *tikvSnapshot) Get(k kv.Key) ([]byte, error) { if err != nil { return nil, errors.Trace(err) } + err = s.store.CheckVisibility(s.version.Ver) + if err != nil { + return nil, errors.Trace(err) + } + if len(val) == 0 { return nil, kv.ErrNotExist } diff --git a/store/tikv/txn.go b/store/tikv/txn.go index 37d7b22a59648..1cd976023bf79 100644 --- a/store/tikv/txn.go +++ b/store/tikv/txn.go @@ -140,11 +140,6 @@ func (txn *tikvTxn) Get(k kv.Key) ([]byte, error) { return nil, errors.Trace(err) } - err = txn.store.CheckVisibility(txn.startTS) - if err != nil { - return nil, errors.Trace(err) - } - return ret, nil } From 84e061ae4b1ae75a9201ecdbf76dbe541c0b2a0b Mon Sep 17 00:00:00 2001 From: crazycs Date: Tue, 14 Jan 2020 10:09:36 +0800 Subject: [PATCH 2/3] add more test --- executor/point_get_test.go | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/executor/point_get_test.go b/executor/point_get_test.go index 8634e1dd3cb6c..38de3e6665933 100644 --- a/executor/point_get_test.go +++ b/executor/point_get_test.go @@ -19,6 +19,7 @@ import ( "time" . "github.com/pingcap/check" + "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/session" @@ -448,11 +449,11 @@ func (s *testPointGetSuite) TestPointGetByRowID(c *C) { tk.MustQuery("select * from t where t._tidb_rowid = 1").Check(testkit.Rows("aaa 12")) } -func (s *testPointGetSuite) TestPointGetCheckVisibility(c *C) { +func (s *testPointGetSuite) TestSelectCheckVisibility(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") tk.MustExec("drop table if exists t") - tk.MustExec("create table t (a varchar(10) key, b int)") + tk.MustExec("create table t (a varchar(10) key, b int,index idx(b))") tk.MustExec("insert into t values('1',1)") tk.MustExec("begin") txn, err := tk.Se.Txn(false) @@ -461,17 +462,21 @@ func (s *testPointGetSuite) TestPointGetCheckVisibility(c *C) { store := tk.Se.GetStore().(tikv.Storage) // Update gc safe time for check data visibility. store.UpdateSPCache(ts+1, time.Now()) + checkSelectResultError := func(sql string, expectErr *terror.Error) { + re, err := tk.Exec(sql) + c.Assert(err, IsNil) + _, err = session.ResultSetToStringSlice(context.Background(), tk.Se, re) + c.Assert(err, NotNil) + c.Assert(expectErr.Equal(err), IsTrue) + } // Test point get. - re, err := tk.Exec("select * from t where a='1'") - c.Assert(err, IsNil) - _, err = session.ResultSetToStringSlice(context.Background(), tk.Se, re) - c.Assert(err, NotNil) - c.Assert(tikv.ErrGCTooEarly.Equal(err), IsTrue) + checkSelectResultError("select * from t where a='1'", tikv.ErrGCTooEarly) // Test batch point get. - re, err = tk.Exec("select * from t where a in ('1','2') ") - c.Assert(err, IsNil) - _, err = session.ResultSetToStringSlice(context.Background(), tk.Se, re) - c.Assert(err, NotNil) - c.Assert(tikv.ErrGCTooEarly.Equal(err), IsTrue) - + checkSelectResultError("select * from t where a in ('1','2')", tikv.ErrGCTooEarly) + // Test Index look up read. + checkSelectResultError("select * from t where b > 0 ", tikv.ErrGCTooEarly) + // Test Index read. + checkSelectResultError("select b from t where b > 0 ", tikv.ErrGCTooEarly) + // Test table read. + checkSelectResultError("select * from t", tikv.ErrGCTooEarly) } From 8730fbf5266911f81a77f2fa3e73115590083a67 Mon Sep 17 00:00:00 2001 From: crazycs Date: Wed, 15 Jan 2020 11:22:39 +0800 Subject: [PATCH 3/3] fix test --- executor/point_get_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/point_get_test.go b/executor/point_get_test.go index 38de3e6665933..8b5cf8bcdc5d8 100644 --- a/executor/point_get_test.go +++ b/executor/point_get_test.go @@ -465,7 +465,7 @@ func (s *testPointGetSuite) TestSelectCheckVisibility(c *C) { checkSelectResultError := func(sql string, expectErr *terror.Error) { re, err := tk.Exec(sql) c.Assert(err, IsNil) - _, err = session.ResultSetToStringSlice(context.Background(), tk.Se, re) + _, err = session.GetRows4Test(context.Background(), tk.Se, re) c.Assert(err, NotNil) c.Assert(expectErr.Equal(err), IsTrue) }