Skip to content

Commit

Permalink
table: fix wrong handle comparation in index double write (#57418)
Browse files Browse the repository at this point in the history
close #57414
  • Loading branch information
joechenrh authored Nov 19, 2024
1 parent d86a366 commit 058d947
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 9 deletions.
9 changes: 1 addition & 8 deletions pkg/ddl/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -1352,6 +1352,7 @@ func doReorgWorkForCreateIndex(
MockDMLExecutionStateBeforeMerge()
}
})
failpoint.InjectCall("BeforeBackfillMerge")
logutil.DDLLogger().Info("index backfill state ready to merge",
zap.Int64("job ID", job.ID),
zap.String("table", tbl.Meta().Name.O),
Expand Down Expand Up @@ -2024,14 +2025,6 @@ func (w *addIndexTxnWorker) checkHandleExists(idxInfo *model.IndexInfo, key kv.K
if hasBeenBackFilled {
return nil
}
if idxInfo.Global {
// 'handle' comes from reading directly from a partition, without partition id,
// so we can only compare the handle part of the key.
if ph, ok := h.(kv.PartitionHandle); ok && ph.Handle.Equal(handle) {
// table row has been back-filled already, OK to add the index entry
return nil
}
}
return ddlutil.GenKeyExistsErr(key, value, idxInfo, tblInfo)
}

Expand Down
26 changes: 26 additions & 0 deletions pkg/ddl/index_modify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1422,3 +1422,29 @@ func TestAddVectorIndexRollback(t *testing.T) {

testfailpoint.Disable(t, "github.com/pingcap/tidb/pkg/ddl/MockCheckVectorIndexProcess")
}

func TestInsertDuplicateBeforeIndexMerge(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk2 := testkit.NewTestKit(t, store)
tk2.MustExec("set @@global.tidb_ddl_enable_fast_reorg = 1")
tk2.MustExec("set @@global.tidb_enable_dist_task=0")

tk.MustExec("use test")
tk2.MustExec("use test")

// Test issue 57414.
testfailpoint.EnableCall(t, "github.com/pingcap/tidb/pkg/ddl/BeforeBackfillMerge", func() {
tk2.MustExec("insert ignore into t values (1, 2), (1, 2) on duplicate key update col1 = 0, col2 = 0")
})

tk.MustExec("drop table if exists t")
tk.MustExec("create table t (col1 int, col2 int, unique index i1(col2) /*T![global_index] GLOBAL */) PARTITION BY HASH (col1) PARTITIONS 2")
tk.MustExec("alter table t add unique index i2(col1, col2)")
tk.MustExec("admin check table t")

tk.MustExec("drop table if exists t")
tk.MustExec("create table t (col1 int, col2 int, unique index i1(col1, col2)) PARTITION BY HASH (col1) PARTITIONS 2")
tk.MustExec("alter table t add unique index i2(col2) /*T![global_index] GLOBAL */")
tk.MustExec("admin check table t")
}
5 changes: 4 additions & 1 deletion pkg/kv/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -723,10 +723,13 @@ func (ph PartitionHandle) Copy() Handle {

// Equal implements the Handle interface.
func (ph PartitionHandle) Equal(h Handle) bool {
// Compare pid and handle if both sides are `PartitionHandle`.
if ph2, ok := h.(PartitionHandle); ok {
return ph.PartitionID == ph2.PartitionID && ph.Handle.Equal(ph2.Handle)
}
return false

// Otherwise, use underlying handle to do comparation.
return ph.Handle.Equal(h)
}

// Compare implements the Handle interface.
Expand Down
8 changes: 8 additions & 0 deletions pkg/kv/key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,14 @@ func TestHandle(t *testing.T) {
assert.Nil(t, err)
assert.Equal(t, "abc", d.GetString())
assert.Equal(t, "{100, abc}", ch.String())

ph1 := NewPartitionHandle(2, ih)
assert.True(t, ph1.Equal(ih))
assert.True(t, ih.Equal(ph1))

ph2 := NewPartitionHandle(1, ch2)
assert.True(t, ph2.Equal(ch2))
assert.True(t, ch2.Equal(ph2))
}

func TestPaddingHandle(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletions pkg/table/tables/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,8 @@ func (c *index) Delete(ctx table.MutateContext, txn kv.Transaction, indexedValue
if err != nil {
return err
}
// The handle passed in may be a `PartitionHandle`,
// so we can't directly do comparation with them.
if !h.Equal(oh) {
okToDelete = false
}
Expand Down

0 comments on commit 058d947

Please sign in to comment.