diff --git a/ddl/index_merge_tmp.go b/ddl/index_merge_tmp.go index 0150f0fb42b4c..1d72002b7fb27 100644 --- a/ddl/index_merge_tmp.go +++ b/ddl/index_merge_tmp.go @@ -51,41 +51,51 @@ func (w *mergeIndexWorker) batchCheckTemporaryUniqueKey(txn kv.Transaction, idxR } for i, key := range w.originIdxKeys { - if val, found := batchVals[string(key)]; found { + keyStr := string(key) + if val, found := batchVals[keyStr]; found { // Found a value in the original index key. - err := checkTempIndexKey(txn, idxRecords[i], val, w.table) + matchDeleted, err := checkTempIndexKey(txn, idxRecords[i], val, w.table) if err != nil { return errors.Trace(err) } + if matchDeleted { + // Delete from batchVals to prevent false-positive duplicate detection. + delete(batchVals, keyStr) + } } else if idxRecords[i].distinct { // The keys in w.batchCheckKeys also maybe duplicate, // so we need to backfill the not found key into `batchVals` map. - batchVals[string(key)] = idxRecords[i].vals + batchVals[keyStr] = idxRecords[i].vals } } return nil } -func checkTempIndexKey(txn kv.Transaction, tmpRec *temporaryIndexRecord, originIdxVal []byte, tblInfo table.Table) error { +// checkTempIndexKey determines whether there is a duplicated index key entry according to value of temp index. +// For non-delete temp record, if the index values mismatch, it is duplicated. +// For delete temp record, we decode the handle from the origin index value and temp index value. +// - if the handles matche, we can delete the index key. +// - otherwise, we further check if the row exists in the table. +func checkTempIndexKey(txn kv.Transaction, tmpRec *temporaryIndexRecord, originIdxVal []byte, tblInfo table.Table) (matchDelete bool, err error) { if !tmpRec.delete { if tmpRec.distinct && !bytes.Equal(originIdxVal, tmpRec.vals) { - return kv.ErrKeyExists + return false, kv.ErrKeyExists } // The key has been found in the original index, skip merging it. tmpRec.skip = true - return nil + return false, nil } // Delete operation. distinct := tablecodec.IndexKVIsUnique(originIdxVal) if !distinct { // For non-distinct key, it is consist of a null value and the handle. // Same as the non-unique indexes, replay the delete operation on non-distinct keys. - return nil + return false, nil } // For distinct index key values, prevent deleting an unexpected index KV in original index. hdInVal, err := tablecodec.DecodeHandleInUniqueIndexValue(originIdxVal, tblInfo.Meta().IsCommonHandle) if err != nil { - return errors.Trace(err) + return false, errors.Trace(err) } if !tmpRec.handle.Equal(hdInVal) { // The inequality means multiple modifications happened in the same key. @@ -96,16 +106,16 @@ func checkTempIndexKey(txn kv.Transaction, tmpRec *temporaryIndexRecord, originI if kv.IsErrNotFound(err) { // The row is deleted, so we can merge the delete operation to the origin index. tmpRec.skip = false - return nil + return false, nil } // Unexpected errors. - return errors.Trace(err) + return false, errors.Trace(err) } // Don't delete the index key if the row exists. tmpRec.skip = true - return nil + return false, nil } - return nil + return true, nil } // temporaryIndexRecord is the record information of an index. diff --git a/ddl/index_merge_tmp_test.go b/ddl/index_merge_tmp_test.go index 8dd5283d6c3de..382b8636ef6c5 100644 --- a/ddl/index_merge_tmp_test.go +++ b/ddl/index_merge_tmp_test.go @@ -911,3 +911,28 @@ func TestAddIndexUpdateUntouchedValues(t *testing.T) { tk.MustExec("admin check table t;") tk.MustQuery("select * from t;").Check(testkit.Rows("1 1 2", "2 1 2")) } + +func TestAddUniqueIndexFalsePositiveDuplicate(t *testing.T) { + store := testkit.CreateMockStore(t) + + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec(`create table t(a bigint DEFAULT '-13202', + b varchar(221) NOT NULL DEFAULT 'dup', + unique key exist_idx(b), + PRIMARY KEY (a));`) + tk.MustExec("INSERT INTO t VALUES (1,'1'), (2,'dup');") + + tk1 := testkit.NewTestKit(t, store) + tk1.MustExec("use test") + ddl.MockDMLExecution = func() { + _, err := tk1.Exec("replace into `t` values (3, 'dup');") + assert.NoError(t, err) + } + err := failpoint.Enable("github.com/pingcap/tidb/ddl/mockDMLExecution", "1*return(true)->return(false)") + require.NoError(t, err) + + tk.MustExec("alter table t add unique index idx(b);") + tk.MustExec("admin check table t;") + failpoint.Disable("github.com/pingcap/tidb/ddl/mockDMLExecution") +} diff --git a/pkg/sessionctx/BUILD.bazel b/pkg/sessionctx/BUILD.bazel deleted file mode 100644 index e69de29bb2d1d..0000000000000