From 058d9479f4ca91be1926b3ed2792c4f4fc749f02 Mon Sep 17 00:00:00 2001
From: Ruihao Chen <ruihao.chen@pingcap.cn>
Date: Tue, 19 Nov 2024 10:44:41 +0800
Subject: [PATCH] table: fix wrong handle comparation in index double write
 (#57418)

close pingcap/tidb#57414
---
 pkg/ddl/index.go             |  9 +--------
 pkg/ddl/index_modify_test.go | 26 ++++++++++++++++++++++++++
 pkg/kv/key.go                |  5 ++++-
 pkg/kv/key_test.go           |  8 ++++++++
 pkg/table/tables/index.go    |  2 ++
 5 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/pkg/ddl/index.go b/pkg/ddl/index.go
index 0a733715625d5..f56b3c26289f4 100644
--- a/pkg/ddl/index.go
+++ b/pkg/ddl/index.go
@@ -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),
@@ -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)
 }
 
diff --git a/pkg/ddl/index_modify_test.go b/pkg/ddl/index_modify_test.go
index b38d05fbffbd5..f7b503ca7d15a 100644
--- a/pkg/ddl/index_modify_test.go
+++ b/pkg/ddl/index_modify_test.go
@@ -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")
+}
diff --git a/pkg/kv/key.go b/pkg/kv/key.go
index b2af3d3e78b2f..8415e8d732513 100644
--- a/pkg/kv/key.go
+++ b/pkg/kv/key.go
@@ -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.
diff --git a/pkg/kv/key_test.go b/pkg/kv/key_test.go
index 0afacd882c829..b7006056e37a5 100644
--- a/pkg/kv/key_test.go
+++ b/pkg/kv/key_test.go
@@ -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) {
diff --git a/pkg/table/tables/index.go b/pkg/table/tables/index.go
index 8a81bd1212138..4516ff8283d72 100644
--- a/pkg/table/tables/index.go
+++ b/pkg/table/tables/index.go
@@ -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
 						}