From c957241eb567638686aae5ad16fc2ed00d77538c Mon Sep 17 00:00:00 2001 From: ngaut Date: Wed, 28 Oct 2015 11:08:48 +0800 Subject: [PATCH 1/4] fix --- store/localstore/kv.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/store/localstore/kv.go b/store/localstore/kv.go index 7d15637a87..8303e7eaa3 100644 --- a/store/localstore/kv.go +++ b/store/localstore/kv.go @@ -176,13 +176,19 @@ func (s *dbStore) tryConditionLockKey(tid uint64, key string, snapshotVal []byte metaKey := codec.EncodeBytes(nil, []byte(key)) currValue, err := s.db.Get(metaKey) - if errors2.ErrorEqual(err, kv.ErrNotExist) || currValue == nil { - // If it's a new key, we won't need to check its version + if errors2.ErrorEqual(err, kv.ErrNotExist) { + s.keysLocked[key] = tid return nil } if err != nil { return errors.Trace(err) } + + // key not exist. + if currValue == nil { + s.keysLocked[key] = tid + return nil + } _, ver, err := codec.DecodeUint(currValue) if err != nil { return errors.Trace(err) @@ -195,7 +201,6 @@ func (s *dbStore) tryConditionLockKey(tid uint64, key string, snapshotVal []byte } s.keysLocked[key] = tid - return nil } From 35d6cde64809812a0b8f2409bb05e948a0f89c3c Mon Sep 17 00:00:00 2001 From: ngaut Date: Wed, 28 Oct 2015 11:44:38 +0800 Subject: [PATCH 2/4] Change error name style --- table/tables/tables.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/table/tables/tables.go b/table/tables/tables.go index e8d90d46b5..6d54c0b53a 100644 --- a/table/tables/tables.go +++ b/table/tables/tables.go @@ -348,13 +348,13 @@ func (t *Table) AddRecord(ctx context.Context, r []interface{}) (recordID int64, if errors2.ErrorEqual(err, kv.ErrKeyExists) { // Get the duplicate row handle // For insert on duplicate syntax, we should update the row - iter, _, terr := v.X.Seek(txn, colVals) - if terr != nil { - return 0, errors.Trace(terr) + iter, _, err1 := v.X.Seek(txn, colVals) + if err1 != nil { + return 0, errors.Trace(err1) } - _, h, terr := iter.Next() - if terr != nil { - return 0, errors.Trace(terr) + _, h, err1 := iter.Next() + if err1 != nil { + return 0, errors.Trace(err1) } return h, errors.Trace(err) } From b39592c02e8e87337a10ffc66051f17dc8d8b4ff Mon Sep 17 00:00:00 2001 From: ngaut Date: Wed, 28 Oct 2015 13:48:45 +0800 Subject: [PATCH 3/4] kv: Do Lock key when create unique index Fix #461 --- kv/index_iter.go | 22 +++++++++++++--------- session.go | 2 +- tidb_test.go | 15 +++++++++++++++ 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/kv/index_iter.go b/kv/index_iter.go index e19004ddeb..d40c8e3af6 100644 --- a/kv/index_iter.go +++ b/kv/index_iter.go @@ -159,20 +159,24 @@ func (c *kvIndex) genIndexKey(indexedValues []interface{}, h int64) ([]byte, err // Create creates a new entry in the kvIndex data. // If the index is unique and there already exists an entry with the same key, Create will return ErrKeyExists func (c *kvIndex) Create(txn Transaction, indexedValues []interface{}, h int64) error { - keyBuf, err := c.genIndexKey(indexedValues, h) + key, err := c.genIndexKey(indexedValues, h) if err != nil { return errors.Trace(err) } if !c.unique { // TODO: reconsider value - err = txn.Set(keyBuf, []byte("timestamp?")) + err = txn.Set(key, []byte("timestamp?")) return errors.Trace(err) } // unique index - _, err = txn.Get(keyBuf) + err = txn.LockKeys(key) + if err != nil { + return errors.Trace(err) + } + _, err = txn.Get(key) if IsErrNotFound(err) { - err = txn.Set(keyBuf, encodeHandle(h)) + err = txn.Set(key, encodeHandle(h)) return errors.Trace(err) } @@ -181,11 +185,11 @@ func (c *kvIndex) Create(txn Transaction, indexedValues []interface{}, h int64) // Delete removes the entry for handle h and indexdValues from KV index. func (c *kvIndex) Delete(txn Transaction, indexedValues []interface{}, h int64) error { - keyBuf, err := c.genIndexKey(indexedValues, h) + key, err := c.genIndexKey(indexedValues, h) if err != nil { return errors.Trace(err) } - err = txn.Delete(keyBuf) + err = txn.Delete(key) return errors.Trace(err) } @@ -217,17 +221,17 @@ func (c *kvIndex) Drop(txn Transaction) error { // Seek searches KV index for the entry with indexedValues. func (c *kvIndex) Seek(txn Transaction, indexedValues []interface{}) (iter IndexIterator, hit bool, err error) { - keyBuf, err := c.genIndexKey(indexedValues, 0) + key, err := c.genIndexKey(indexedValues, 0) if err != nil { return nil, false, errors.Trace(err) } - it, err := txn.Seek(keyBuf) + it, err := txn.Seek(key) if err != nil { return nil, false, errors.Trace(err) } // check if hit hit = false - if it.Valid() && it.Key() == string(keyBuf) { + if it.Valid() && it.Key() == string(key) { hit = true } return &indexIter{it: it, idx: c, prefix: c.prefix}, hit, nil diff --git a/session.go b/session.go index 8e4adf2cac..44a2a3c580 100644 --- a/session.go +++ b/session.go @@ -256,7 +256,7 @@ func (s *session) ExecRestrictedSQL(ctx context.Context, sql string) (rset.Recor // Check statement for some restriction // For example only support DML on system meta table. // TODO: Add more restrictions. - log.Infof("Executing %s [%s]", st, sql) + log.Infof("Executing %s [%s]", st.OriginText(), sql) ctx.SetValue(&sqlexec.RestrictedSQLExecutorKeyType{}, true) defer ctx.ClearValue(&sqlexec.RestrictedSQLExecutorKeyType{}) rs, err := st.Exec(ctx) diff --git a/tidb_test.go b/tidb_test.go index 3aaa755704..fa6d38223b 100644 --- a/tidb_test.go +++ b/tidb_test.go @@ -1245,6 +1245,21 @@ func (s *testSessionSuite) TestResultType(c *C) { c.Assert(fs[0].Col.FieldType.Tp, Equals, mysql.TypeString) } +func (s *testSessionSuite) TestIssue461(c *C) { + // Testcase for https://github.com/pingcap/tidb/issues/325 + store := newStore(c, s.dbName) + se1 := newSession(c, store, s.dbName) + mustExecSQL(c, se1, + `CREATE TABLE test ( id int(11) UNSIGNED NOT NULL AUTO_INCREMENT, val int UNIQUE, PRIMARY KEY (id)); `) + mustExecSQL(c, se1, "begin;") + mustExecSQL(c, se1, "insert into test(id, val) values(1, 1);") + se2 := newSession(c, store, s.dbName) + mustExecSQL(c, se2, "begin;") + mustExecSQL(c, se2, "insert into test(id, val) values(1, 1);") + mustExecSQL(c, se2, "commit;") + mustExecFailed(c, se1, "commit;") +} + func (s *testSessionSuite) TestBuiltin(c *C) { store := newStore(c, s.dbName) se := newSession(c, store, s.dbName) From 3a1d4e01d92de134c5fde64631e7b84ab905ced3 Mon Sep 17 00:00:00 2001 From: ngaut Date: Wed, 28 Oct 2015 14:02:41 +0800 Subject: [PATCH 4/4] clean up --- tidb_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tidb_test.go b/tidb_test.go index fa6d38223b..1edcecf228 100644 --- a/tidb_test.go +++ b/tidb_test.go @@ -1246,7 +1246,6 @@ func (s *testSessionSuite) TestResultType(c *C) { } func (s *testSessionSuite) TestIssue461(c *C) { - // Testcase for https://github.com/pingcap/tidb/issues/325 store := newStore(c, s.dbName) se1 := newSession(c, store, s.dbName) mustExecSQL(c, se1, @@ -1258,6 +1257,9 @@ func (s *testSessionSuite) TestIssue461(c *C) { mustExecSQL(c, se2, "insert into test(id, val) values(1, 1);") mustExecSQL(c, se2, "commit;") mustExecFailed(c, se1, "commit;") + + se := newSession(c, store, s.dbName) + mustExecSQL(c, se, "drop table test;") } func (s *testSessionSuite) TestBuiltin(c *C) {