From 9a3f362639ddeb837246fd9f5d821f49d495b516 Mon Sep 17 00:00:00 2001 From: Ti Chi Robot Date: Fri, 3 Mar 2023 13:09:10 +0800 Subject: [PATCH] ddl: never write untouched index values to temp index (#41879) (#41884) close pingcap/tidb#41880 --- ddl/index_merge_tmp_test.go | 41 +++++++++++++++++++++++++++++++++++++ ddl/ingest/engine.go | 2 +- table/tables/index.go | 13 +++++++----- 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/ddl/index_merge_tmp_test.go b/ddl/index_merge_tmp_test.go index 42cf75bb4a2cd..8dd5283d6c3de 100644 --- a/ddl/index_merge_tmp_test.go +++ b/ddl/index_merge_tmp_test.go @@ -870,3 +870,44 @@ func TestAddIndexMultipleDelete(t *testing.T) { tk.MustQuery("select * from t;").Check(testkit.Rows()) require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/ddl/mockDMLExecution")) } + +func TestAddIndexUpdateUntouchedValues(t *testing.T) { + store, dom := testkit.CreateMockStoreAndDomain(t) + + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("create table t(id int primary key, b int, k int);") + tk.MustExec("insert into t values (1, 1, 1);") + + tk1 := testkit.NewTestKit(t, store) + tk1.MustExec("use test") + + d := dom.DDL() + originalCallback := d.GetHook() + defer d.SetHook(originalCallback) + callback := &ddl.TestDDLCallback{} + var runDML bool + callback.OnJobRunBeforeExported = func(job *model.Job) { + if t.Failed() || runDML { + return + } + switch job.SchemaState { + case model.StateWriteReorganization: + _, err := tk1.Exec("begin;") + assert.NoError(t, err) + _, err = tk1.Exec("update t set k=k+1 where id = 1;") + assert.NoError(t, err) + _, err = tk1.Exec("insert into t values (2, 1, 2);") + // Should not report "invalid temp index value". + assert.NoError(t, err) + _, err = tk1.Exec("commit;") + assert.NoError(t, err) + runDML = true + } + } + d.SetHook(callback) + + tk.MustGetErrCode("alter table t add unique index idx(b);", errno.ErrDupEntry) + tk.MustExec("admin check table t;") + tk.MustQuery("select * from t;").Check(testkit.Rows("1 1 2", "2 1 2")) +} diff --git a/ddl/ingest/engine.go b/ddl/ingest/engine.go index ac0287b26637a..fce350df09e40 100644 --- a/ddl/ingest/engine.go +++ b/ddl/ingest/engine.go @@ -204,7 +204,7 @@ func (ei *engineInfo) newWriterContext(workerID int, unique bool) (*WriterContex func (ei *engineInfo) closeWriters() error { var firstErr error - for wid := range ei.writerCache.Keys() { + for _, wid := range ei.writerCache.Keys() { if w, ok := ei.writerCache.Load(wid); ok { _, err := w.Close(ei.ctx) if err != nil { diff --git a/table/tables/index.go b/table/tables/index.go index 4b10605535c8f..b8a164a9b2ece 100644 --- a/table/tables/index.go +++ b/table/tables/index.go @@ -177,7 +177,12 @@ func (c *index) Create(sctx sessionctx.Context, txn kv.Transaction, indexedValue if !distinct || skipCheck || opt.Untouched { val := idxVal - if keyIsTempIdxKey && !opt.Untouched { // Untouched key-values never occur in the storage. + if opt.Untouched && (keyIsTempIdxKey || len(tempKey) > 0) { + // Untouched key-values never occur in the storage and the temp index is not public. + // It is unnecessary to write the untouched temp index key-values. + return nil, nil + } + if keyIsTempIdxKey { tempVal := tablecodec.TempIndexValueElem{Value: idxVal, KeyVer: keyVer, Distinct: distinct} val = tempVal.Encode(nil) } @@ -186,10 +191,8 @@ func (c *index) Create(sctx sessionctx.Context, txn kv.Transaction, indexedValue return nil, err } if len(tempKey) > 0 { - if !opt.Untouched { // Untouched key-values never occur in the storage. - tempVal := tablecodec.TempIndexValueElem{Value: idxVal, KeyVer: keyVer, Distinct: distinct} - val = tempVal.Encode(nil) - } + tempVal := tablecodec.TempIndexValueElem{Value: idxVal, KeyVer: keyVer, Distinct: distinct} + val = tempVal.Encode(nil) err = txn.GetMemBuffer().Set(tempKey, val) if err != nil { return nil, err