From 7adf7c12e6f484db745357593087c617ece2d11a Mon Sep 17 00:00:00 2001 From: xiongjiwei Date: Mon, 6 Feb 2023 14:25:30 +0800 Subject: [PATCH 1/4] fix recover expression index Signed-off-by: xiongjiwei --- executor/admin.go | 43 +++++++++++++++++++++++++++++++++++++- executor/builder.go | 50 ++++++++++++++++++++++++++++++--------------- 2 files changed, 75 insertions(+), 18 deletions(-) diff --git a/executor/admin.go b/executor/admin.go index 7fc5ad37fb31e..e73ca6503755b 100644 --- a/executor/admin.go +++ b/executor/admin.go @@ -16,6 +16,7 @@ package executor import ( "context" + "github.com/pingcap/tidb/expression" "math" "github.com/pingcap/errors" @@ -191,6 +192,9 @@ type RecoverIndexExec struct { srcChunk *chunk.Chunk handleCols plannercore.HandleCols + containsGenedCol bool + cols []*expression.Column + // below buf is used to reduce allocations. recoverRows []recoverRows idxValsBufs [][]types.Datum @@ -379,7 +383,10 @@ func (e *RecoverIndexExec) fetchRecoverRows(ctx context.Context, srcResult dists if err != nil { return nil, err } - idxVals := extractIdxVals(row, e.idxValsBufs[result.scanRowCount], e.colFieldTypes, idxValLen) + idxVals, err := e.buildIndexedValues(row, e.idxValsBufs[result.scanRowCount], e.colFieldTypes, idxValLen) + if err != nil { + return nil, err + } e.idxValsBufs[result.scanRowCount] = idxVals rsData := tables.TryGetHandleRestoredDataWrapper(e.table.Meta(), plannercore.GetCommonHandleDatum(e.handleCols, row), nil, e.index.Meta()) e.recoverRows = append(e.recoverRows, recoverRows{handle: handle, idxVals: idxVals, rsData: rsData, skip: false}) @@ -391,6 +398,40 @@ func (e *RecoverIndexExec) fetchRecoverRows(ctx context.Context, srcResult dists return e.recoverRows, nil } +func (e *RecoverIndexExec) buildIndexedValues(row chunk.Row, idxVals []types.Datum, fieldTypes []*types.FieldType, idxValLen int) ([]types.Datum, error) { + if !e.containsGenedCol { + return extractIdxVals(row, idxVals, fieldTypes, idxValLen), nil + } + + if e.cols == nil { + columns, _, err := expression.ColumnInfos2ColumnsAndNames(e.ctx, model.NewCIStr("mock"), e.table.Meta().Name, e.table.Meta().Columns, e.table.Meta()) + if err != nil { + return nil, err + } + e.cols = columns + } + + if cap(idxVals) < idxValLen { + idxVals = make([]types.Datum, idxValLen) + } else { + idxVals = idxVals[:idxValLen] + } + + for i, col := range e.index.Meta().Columns { + if e.table.Meta().Columns[col.Offset].IsGenerated() { + val, err := e.cols[col.Offset].EvalVirtualColumn(row) + if err != nil { + return nil, err + } + val.Copy(&idxVals[i]) + } else { + val := row.GetDatum(col.Offset, &(e.table.Meta().Columns[col.Offset].FieldType)) + val.Copy(&idxVals[i]) + } + } + return idxVals, nil +} + func (e *RecoverIndexExec) batchMarkDup(txn kv.Transaction, rows []recoverRows) error { if len(rows) == 0 { return nil diff --git a/executor/builder.go b/executor/builder.go index 3ebb0e4f3a1e9..d579bb0b98a4d 100644 --- a/executor/builder.go +++ b/executor/builder.go @@ -481,18 +481,14 @@ func (b *executorBuilder) buildCheckTable(v *plannercore.CheckTable) Executor { return e } -func buildIdxColsConcatHandleCols(tblInfo *model.TableInfo, indexInfo *model.IndexInfo) []*model.ColumnInfo { - handleLen := 1 +func buildIdxColsConcatHandleCols(tblInfo *model.TableInfo, indexInfo *model.IndexInfo, f func(indexInfo *model.IndexInfo) []*model.ColumnInfo) []*model.ColumnInfo { var pkCols []*model.IndexColumn if tblInfo.IsCommonHandle { pkIdx := tables.FindPrimaryIndex(tblInfo) pkCols = pkIdx.Columns - handleLen = len(pkIdx.Columns) - } - columns := make([]*model.ColumnInfo, 0, len(indexInfo.Columns)+handleLen) - for _, idxCol := range indexInfo.Columns { - columns = append(columns, tblInfo.Columns[idxCol.Offset]) } + + columns := f(indexInfo) if tblInfo.IsCommonHandle { for _, c := range pkCols { columns = append(columns, tblInfo.Columns[c.Offset]) @@ -523,12 +519,26 @@ func (b *executorBuilder) buildRecoverIndex(v *plannercore.RecoverIndex) Executo b.err = errors.Errorf("secondary index `%v` is not found in table `%v`", v.IndexName, v.Table.Name.O) return nil } + var containsGenedCol bool + cols := buildIdxColsConcatHandleCols(tblInfo, index.Meta(), func(indexInfo *model.IndexInfo) []*model.ColumnInfo { + columns := make([]*model.ColumnInfo, 0, len(indexInfo.Columns)+8) + for _, iCol := range indexInfo.Columns { + if tblInfo.Columns[iCol.Offset].IsGenerated() { + containsGenedCol = true + return tblInfo.Columns + } + columns = append(columns, tblInfo.Columns[iCol.Offset]) + } + + return columns + }) e := &RecoverIndexExec{ - baseExecutor: newBaseExecutor(b.ctx, v.Schema(), v.ID()), - columns: buildIdxColsConcatHandleCols(tblInfo, index.Meta()), - index: index, - table: t, - physicalID: t.Meta().ID, + baseExecutor: newBaseExecutor(b.ctx, v.Schema(), v.ID()), + columns: cols, + containsGenedCol: containsGenedCol, + index: index, + table: t, + physicalID: t.Meta().ID, } sessCtx := e.ctx.GetSessionVars().StmtCtx e.handleCols = buildHandleColsForExec(sessCtx, tblInfo, index.Meta(), e.columns) @@ -585,11 +595,17 @@ func (b *executorBuilder) buildCleanupIndex(v *plannercore.CleanupIndex) Executo } e := &CleanupIndexExec{ baseExecutor: newBaseExecutor(b.ctx, v.Schema(), v.ID()), - columns: buildIdxColsConcatHandleCols(tblInfo, index.Meta()), - index: index, - table: t, - physicalID: t.Meta().ID, - batchSize: 20000, + columns: buildIdxColsConcatHandleCols(tblInfo, index.Meta(), func(indexInfo *model.IndexInfo) []*model.ColumnInfo { + columns := make([]*model.ColumnInfo, 0, len(indexInfo.Columns)+8) + for _, idxCol := range indexInfo.Columns { + columns = append(columns, tblInfo.Columns[idxCol.Offset]) + } + return columns + }), + index: index, + table: t, + physicalID: t.Meta().ID, + batchSize: 20000, } sessCtx := e.ctx.GetSessionVars().StmtCtx e.handleCols = buildHandleColsForExec(sessCtx, tblInfo, index.Meta(), e.columns) From c19ec3af3f3c286806b281b01bf05d80898fe2d6 Mon Sep 17 00:00:00 2001 From: xiongjiwei Date: Mon, 6 Feb 2023 15:05:21 +0800 Subject: [PATCH 2/4] add tests Signed-off-by: xiongjiwei --- executor/admin_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/executor/admin_test.go b/executor/admin_test.go index c91cd343718db..07afb7b172d72 100644 --- a/executor/admin_test.go +++ b/executor/admin_test.go @@ -302,6 +302,35 @@ func TestAdminRecoverIndex(t *testing.T) { tk.MustExec("admin check index admin_test c2") tk.MustExec("admin check table admin_test") + + tk.MustExec("drop table if exists admin_test") + tk.MustExec("create table admin_test (c1 int, c2 int, c3 int default 1, primary key(c1), unique key i1((c2+1)))") + tk.MustExec("insert admin_test (c1, c2) values (1, 1), (2, 2), (3, 3), (10, 10), (20, 20)") + r = tk.MustQuery("admin recover index admin_test i1") + r.Check(testkit.Rows("0 5")) + tk.MustExec("admin check table admin_test") + ctx.Store = store + is = domain.InfoSchema() + dbName = model.NewCIStr("test") + tblName = model.NewCIStr("admin_test") + tbl, err = is.TableByName(dbName, tblName) + require.NoError(t, err) + + tblInfo = tbl.Meta() + idxInfo = tblInfo.FindIndexByName("i1") + indexOpr = tables.NewIndex(tblInfo.ID, tblInfo, idxInfo) + sc = ctx.GetSessionVars().StmtCtx + txn, err = store.Begin() + require.NoError(t, err) + err = indexOpr.Delete(sc, txn, types.MakeDatums(1), kv.IntHandle(1)) + require.NoError(t, err) + err = txn.Commit(context.Background()) + require.NoError(t, err) + err = tk.ExecToErr("admin check table admin_test") + require.Error(t, err) + r = tk.MustQuery("admin recover index admin_test i1") + r.Check(testkit.Rows("1 5")) + tk.MustExec("admin check table admin_test") } func TestAdminCleanupMVIndex(t *testing.T) { From 6bbba129a641a1a807e385c69efc7b5f6d93e4d7 Mon Sep 17 00:00:00 2001 From: xiongjiwei Date: Mon, 6 Feb 2023 16:27:18 +0800 Subject: [PATCH 3/4] add tests and fix bug Signed-off-by: xiongjiwei --- executor/admin.go | 2 +- executor/admin_test.go | 10 +++++++++- executor/builder.go | 7 +++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/executor/admin.go b/executor/admin.go index e73ca6503755b..4000111998c37 100644 --- a/executor/admin.go +++ b/executor/admin.go @@ -16,12 +16,12 @@ package executor import ( "context" - "github.com/pingcap/tidb/expression" "math" "github.com/pingcap/errors" "github.com/pingcap/kvproto/pkg/kvrpcpb" "github.com/pingcap/tidb/distsql" + "github.com/pingcap/tidb/expression" "github.com/pingcap/tidb/infoschema" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/parser/ast" diff --git a/executor/admin_test.go b/executor/admin_test.go index 07afb7b172d72..95b14bb27312d 100644 --- a/executor/admin_test.go +++ b/executor/admin_test.go @@ -309,6 +309,7 @@ func TestAdminRecoverIndex(t *testing.T) { r = tk.MustQuery("admin recover index admin_test i1") r.Check(testkit.Rows("0 5")) tk.MustExec("admin check table admin_test") + ctx = mock.NewContext() ctx.Store = store is = domain.InfoSchema() dbName = model.NewCIStr("test") @@ -322,15 +323,22 @@ func TestAdminRecoverIndex(t *testing.T) { sc = ctx.GetSessionVars().StmtCtx txn, err = store.Begin() require.NoError(t, err) - err = indexOpr.Delete(sc, txn, types.MakeDatums(1), kv.IntHandle(1)) + err = indexOpr.Delete(sc, txn, types.MakeDatums(2), kv.IntHandle(1)) require.NoError(t, err) err = txn.Commit(context.Background()) require.NoError(t, err) + r = tk.MustQuery("SELECT COUNT(*) FROM admin_test USE INDEX(i1)") + r.Check(testkit.Rows("4")) err = tk.ExecToErr("admin check table admin_test") require.Error(t, err) r = tk.MustQuery("admin recover index admin_test i1") r.Check(testkit.Rows("1 5")) tk.MustExec("admin check table admin_test") + + tk.MustExec("drop table if exists admin_test") + tk.MustExec("create table admin_test (c1 int, c2 int, c3 int default 1, primary key(c1), unique key i1(c1, c2));") + tk.MustExec("insert admin_test (c1, c2) values (1, 1), (2, 2), (3, 3), (10, 10), (20, 20);") + tk.MustExec("admin recover index admin_test i1;") } func TestAdminCleanupMVIndex(t *testing.T) { diff --git a/executor/builder.go b/executor/builder.go index d579bb0b98a4d..42b3efc968086 100644 --- a/executor/builder.go +++ b/executor/builder.go @@ -495,6 +495,10 @@ func buildIdxColsConcatHandleCols(tblInfo *model.TableInfo, indexInfo *model.Ind } return columns } + if tblInfo.PKIsHandle { + columns = append(columns, tblInfo.Columns[tblInfo.GetPkColInfo().Offset]) + return columns + } handleOffset := len(columns) handleColsInfo := &model.ColumnInfo{ ID: model.ExtraHandleID, @@ -527,6 +531,9 @@ func (b *executorBuilder) buildRecoverIndex(v *plannercore.RecoverIndex) Executo containsGenedCol = true return tblInfo.Columns } + if tblInfo.PKIsHandle && tblInfo.GetPkColInfo().Offset == iCol.Offset { + continue + } columns = append(columns, tblInfo.Columns[iCol.Offset]) } From 52c621922a51c23f1aa6dfcd624d31ef297753dd Mon Sep 17 00:00:00 2001 From: xiongjiwei Date: Tue, 7 Feb 2023 11:05:06 +0800 Subject: [PATCH 4/4] refactor Signed-off-by: xiongjiwei --- executor/builder.go | 54 +++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/executor/builder.go b/executor/builder.go index 42b3efc968086..12cdeeaaa44ef 100644 --- a/executor/builder.go +++ b/executor/builder.go @@ -481,14 +481,25 @@ func (b *executorBuilder) buildCheckTable(v *plannercore.CheckTable) Executor { return e } -func buildIdxColsConcatHandleCols(tblInfo *model.TableInfo, indexInfo *model.IndexInfo, f func(indexInfo *model.IndexInfo) []*model.ColumnInfo) []*model.ColumnInfo { +func buildIdxColsConcatHandleCols(tblInfo *model.TableInfo, indexInfo *model.IndexInfo, hasGenedCol bool) []*model.ColumnInfo { var pkCols []*model.IndexColumn if tblInfo.IsCommonHandle { pkIdx := tables.FindPrimaryIndex(tblInfo) pkCols = pkIdx.Columns } - columns := f(indexInfo) + columns := make([]*model.ColumnInfo, 0, len(indexInfo.Columns)+len(pkCols)) + if hasGenedCol { + columns = tblInfo.Columns + } else { + for _, idxCol := range indexInfo.Columns { + if tblInfo.PKIsHandle && tblInfo.GetPkColInfo().Offset == idxCol.Offset { + continue + } + columns = append(columns, tblInfo.Columns[idxCol.Offset]) + } + } + if tblInfo.IsCommonHandle { for _, c := range pkCols { columns = append(columns, tblInfo.Columns[c.Offset]) @@ -523,26 +534,17 @@ func (b *executorBuilder) buildRecoverIndex(v *plannercore.RecoverIndex) Executo b.err = errors.Errorf("secondary index `%v` is not found in table `%v`", v.IndexName, v.Table.Name.O) return nil } - var containsGenedCol bool - cols := buildIdxColsConcatHandleCols(tblInfo, index.Meta(), func(indexInfo *model.IndexInfo) []*model.ColumnInfo { - columns := make([]*model.ColumnInfo, 0, len(indexInfo.Columns)+8) - for _, iCol := range indexInfo.Columns { - if tblInfo.Columns[iCol.Offset].IsGenerated() { - containsGenedCol = true - return tblInfo.Columns - } - if tblInfo.PKIsHandle && tblInfo.GetPkColInfo().Offset == iCol.Offset { - continue - } - columns = append(columns, tblInfo.Columns[iCol.Offset]) + var hasGenedCol bool + for _, iCol := range index.Meta().Columns { + if tblInfo.Columns[iCol.Offset].IsGenerated() { + hasGenedCol = true } - - return columns - }) + } + cols := buildIdxColsConcatHandleCols(tblInfo, index.Meta(), hasGenedCol) e := &RecoverIndexExec{ baseExecutor: newBaseExecutor(b.ctx, v.Schema(), v.ID()), columns: cols, - containsGenedCol: containsGenedCol, + containsGenedCol: hasGenedCol, index: index, table: t, physicalID: t.Meta().ID, @@ -602,17 +604,11 @@ func (b *executorBuilder) buildCleanupIndex(v *plannercore.CleanupIndex) Executo } e := &CleanupIndexExec{ baseExecutor: newBaseExecutor(b.ctx, v.Schema(), v.ID()), - columns: buildIdxColsConcatHandleCols(tblInfo, index.Meta(), func(indexInfo *model.IndexInfo) []*model.ColumnInfo { - columns := make([]*model.ColumnInfo, 0, len(indexInfo.Columns)+8) - for _, idxCol := range indexInfo.Columns { - columns = append(columns, tblInfo.Columns[idxCol.Offset]) - } - return columns - }), - index: index, - table: t, - physicalID: t.Meta().ID, - batchSize: 20000, + columns: buildIdxColsConcatHandleCols(tblInfo, index.Meta(), false), + index: index, + table: t, + physicalID: t.Meta().ID, + batchSize: 20000, } sessCtx := e.ctx.GetSessionVars().StmtCtx e.handleCols = buildHandleColsForExec(sessCtx, tblInfo, index.Meta(), e.columns)