Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ddl: fix unexpect fail when create expression index #39822

Merged
merged 9 commits into from
Dec 13, 2022
Merged

ddl: fix unexpect fail when create expression index #39822

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d18cf71
ddl: fix unexpect fail when create expression index
Defined2014 Dec 12, 2022
3e79821
add test
Defined2014 Dec 12, 2022
f033e63
update test case
Defined2014 Dec 12, 2022
425de57
:Revert "ddl: fix unexpect fail when create expression index"
Defined2014 Dec 12, 2022
587ef63
updated
Defined2014 Dec 12, 2022
857d966
Merge branch 'master' into fix-39784
Defined2014 Dec 12, 2022
0bb5ab4
Merge branch 'master' into fix-39784
ti-chi-bot Dec 13, 2022
dd873e5
Merge branch 'master' into fix-39784
ti-chi-bot Dec 13, 2022
4dc5ce7
Merge branch 'master' into fix-39784
ti-chi-bot Dec 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion ddl/backfilling.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
ddlutil "github.com/pingcap/tidb/ddl/util"
"github.com/pingcap/tidb/expression"
"github.com/pingcap/tidb/kv"
"github.com/pingcap/tidb/meta"
"github.com/pingcap/tidb/metrics"
"github.com/pingcap/tidb/parser/model"
"github.com/pingcap/tidb/parser/terror"
Expand Down Expand Up @@ -805,12 +806,21 @@ func (b *backfillScheduler) Close() {
//
// The above operations are completed in a transaction.
// Finally, update the concurrent processing of the total number of rows, and store the completed handle value.
func (dc *ddlCtx) writePhysicalTableRecord(sessPool *sessionPool, t table.PhysicalTable, bfWorkerType backfillWorkerType, reorgInfo *reorgInfo) error {
func (dc *ddlCtx) writePhysicalTableRecord(sess *session, sessPool *sessionPool, t table.PhysicalTable, bfWorkerType backfillWorkerType, reorgInfo *reorgInfo) error {
job := reorgInfo.Job
totalAddedCount := job.GetRowCount()

startKey, endKey := reorgInfo.StartKey, reorgInfo.EndKey
sessCtx := newContext(reorgInfo.d.store)
m, err := sess.Txn(true)
if err != nil {
return err
}
dbInfo, err := meta.NewMeta(m).GetDatabase(job.SchemaID)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we put it to onCreateIndex which we needn't do more than once? It can be passed through reorgInfo. And we needn't GetDatabase in this background.

Copy link
Contributor Author

@Defined2014 Defined2014 Dec 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR also has other problems like

CREATE TABLE `t2` (
`id` int(11) NOT NULL AUTO_INCREMENT ,
`name` varchar(30) DEFAULT NULL ,
`create_time` datetime NOT NULL ,
PRIMARY KEY (`id`) ,
KEY `idx_create_time` (`create_time`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin;
create index idx_t2_name on test.t2((lower(test.t2.name)));
alter table t2 rename to t3;
insert into t3 values (0, '123', 2022-12-12);

It will failed when execute insert.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, opened another issue to trace rename issue, #39826. PTAL @zimulala

if err != nil {
return err
}
sessCtx.GetSessionVars().CurrentDB = dbInfo.Name.O
decodeColMap, err := makeupDecodeColMap(sessCtx, t)
if err != nil {
return errors.Trace(err)
Expand Down
4 changes: 2 additions & 2 deletions ddl/column.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1047,7 +1047,7 @@ func (w *worker) updatePhysicalTableRow(t table.Table, reorgInfo *reorgInfo) err
if p == nil {
return dbterror.ErrCancelledDDLJob.GenWithStack("Can not find partition id %d for table %d", reorgInfo.PhysicalTableID, t.Meta().ID)
}
err := w.writePhysicalTableRecord(w.sessPool, p, typeUpdateColumnWorker, reorgInfo)
err := w.writePhysicalTableRecord(w.sess, w.sessPool, p, typeUpdateColumnWorker, reorgInfo)
if err != nil {
return err
}
Expand All @@ -1059,7 +1059,7 @@ func (w *worker) updatePhysicalTableRow(t table.Table, reorgInfo *reorgInfo) err
return nil
}
if tbl, ok := t.(table.PhysicalTable); ok {
return w.writePhysicalTableRecord(w.sessPool, tbl, typeUpdateColumnWorker, reorgInfo)
return w.writePhysicalTableRecord(w.sess, w.sessPool, tbl, typeUpdateColumnWorker, reorgInfo)
}
return dbterror.ErrCancelledDDLJob.GenWithStack("internal error for phys tbl id: %d tbl id: %d", reorgInfo.PhysicalTableID, t.Meta().ID)
}
Expand Down
6 changes: 6 additions & 0 deletions ddl/db_change_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1712,6 +1712,12 @@ func TestCreateExpressionIndex(t *testing.T) {
require.NoError(t, checkErr)
tk.MustExec("admin check table t")
tk.MustQuery("select * from t order by a, b").Check(testkit.Rows("0 9", "0 11", "0 11", "1 7", "2 7", "5 7", "8 8", "10 10", "10 10"))

// https://github.com/pingcap/tidb/issues/39784
tk.MustExec("use test")
tk.MustExec("drop table if exists t")
tk.MustExec("create table t(name varchar(20))")
tk.MustExec("create index idx on test.t((lower(test.t.name)))")
}

func TestCreateUniqueExpressionIndex(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions ddl/index.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1592,10 +1592,10 @@ func (w *addIndexWorker) BackfillDataInTxn(handleRange reorgBackfillTask) (taskC
func (w *worker) addPhysicalTableIndex(t table.PhysicalTable, reorgInfo *reorgInfo) error {
if reorgInfo.mergingTmpIdx {
logutil.BgLogger().Info("[ddl] start to merge temp index", zap.String("job", reorgInfo.Job.String()), zap.String("reorgInfo", reorgInfo.String()))
return w.writePhysicalTableRecord(w.sessPool, t, typeAddIndexMergeTmpWorker, reorgInfo)
return w.writePhysicalTableRecord(w.sess, w.sessPool, t, typeAddIndexMergeTmpWorker, reorgInfo)
}
logutil.BgLogger().Info("[ddl] start to add table index", zap.String("job", reorgInfo.Job.String()), zap.String("reorgInfo", reorgInfo.String()))
return w.writePhysicalTableRecord(w.sessPool, t, typeAddIndexWorker, reorgInfo)
return w.writePhysicalTableRecord(w.sess, w.sessPool, t, typeAddIndexWorker, reorgInfo)
}

// addTableIndex handles the add index reorganization state for a table.
Expand Down Expand Up @@ -1804,7 +1804,7 @@ func (w *cleanUpIndexWorker) BackfillDataInTxn(handleRange reorgBackfillTask) (t
// cleanupPhysicalTableIndex handles the drop partition reorganization state for a non-partitioned table or a partition.
func (w *worker) cleanupPhysicalTableIndex(t table.PhysicalTable, reorgInfo *reorgInfo) error {
logutil.BgLogger().Info("[ddl] start to clean up index", zap.String("job", reorgInfo.Job.String()), zap.String("reorgInfo", reorgInfo.String()))
return w.writePhysicalTableRecord(w.sessPool, t, typeCleanUpIndexWorker, reorgInfo)
return w.writePhysicalTableRecord(w.sess, w.sessPool, t, typeCleanUpIndexWorker, reorgInfo)
}

// cleanupGlobalIndex handles the drop partition reorganization state to clean up index entries of partitions.
Expand Down