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

executor: skip ignore check for not update indexes #23894

Merged
merged 6 commits into from
Apr 12, 2021
Merged

executor: skip ignore check for not update indexes #23894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cdfd9ca
executor: skip ignore check for not update indexes
lysu Apr 7, 2021
a74efaa
address comments
lysu Apr 7, 2021
729db71
...
lysu Apr 7, 2021
07d4a28
fix..
lysu Apr 7, 2021
02c28cb
address comments
lysu Apr 12, 2021
3eb3432
Merge branch 'master' into fix-insert-ignore-no-change
ti-chi-bot Apr 12, 2021
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
2 changes: 1 addition & 1 deletion executor/write.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func updateRecord(ctx context.Context, sctx sessionctx.Context, h kv.Handle, old
if sc.DupKeyAsWarning {
// For `UPDATE IGNORE`/`INSERT IGNORE ON DUPLICATE KEY UPDATE`
// If the new handle or unique index exists, this will avoid to remove the record.
err = tables.CheckHandleOrUniqueKeyExistForUpdateIgnoreOrInsertOnDupIgnore(ctx, sctx, t, newHandle, newData)
err = tables.CheckHandleOrUniqueKeyExistForUpdateIgnoreOrInsertOnDupIgnore(ctx, sctx, t, newHandle, newData, modified)
if err != nil {
if terr, ok := errors.Cause(err).(*terror.Error); sctx.GetSessionVars().StmtCtx.IgnoreNoPartition && ok && terr.Code() == errno.ErrNoPartitionForGivenValue {
return false, nil
Expand Down
8 changes: 8 additions & 0 deletions executor/write_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,14 @@ func (s *testSuite4) TestInsertIgnoreOnDup(c *C) {
tk.MustExec("update ignore t5 set k2 = '2', uk1 = 2 where k1 = '1' and k2 = '1'")
tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1062 Duplicate entry '2' for key 'ukk1'"))
tk.MustQuery("select * from t5").Check(testkit.Rows("1 1 1 100", "1 3 2 200"))

tk.MustExec("drop table if exists t6")
tk.MustExec("create table t6 (a int, b int, c int, primary key(a, b) clustered, unique key idx_14(b), unique key idx_15(b), unique key idx_16(a, b))")
tk.MustExec("insert into t6 select 10, 10, 20")
tk.MustExec("insert ignore into t6 set a = 20, b = 10 on duplicate key update a = 100")
tk.MustQuery("select * from t6").Check(testkit.Rows("100 10 20"))
tk.MustExec("insert ignore into t6 set a = 200, b= 10 on duplicate key update c = 1000")
tk.MustQuery("select * from t6").Check(testkit.Rows("100 10 1000"))
}

func (s *testSuite4) TestInsertSetWithDefault(c *C) {
Expand Down
29 changes: 19 additions & 10 deletions table/tables/tables.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1450,11 +1450,11 @@ func FindIndexByColName(t table.Table, name string) table.Index {

// CheckHandleOrUniqueKeyExistForUpdateIgnoreOrInsertOnDupIgnore check whether recordID key or unique index key exists. if not exists, return nil,
// otherwise return kv.ErrKeyExists error.
func CheckHandleOrUniqueKeyExistForUpdateIgnoreOrInsertOnDupIgnore(ctx context.Context, sctx sessionctx.Context, t table.Table, recordID kv.Handle, data []types.Datum) error {
func CheckHandleOrUniqueKeyExistForUpdateIgnoreOrInsertOnDupIgnore(ctx context.Context, sctx sessionctx.Context, t table.Table, recordID kv.Handle, newRow []types.Datum, modified []bool) error {
physicalTableID := t.Meta().ID
if pt, ok := t.(*partitionedTable); ok {
info := t.Meta().GetPartitionInfo()
pid, err := pt.locatePartition(sctx, info, data)
pid, err := pt.locatePartition(sctx, info, newRow)
if err != nil {
return err
}
Expand All @@ -1472,7 +1472,7 @@ func CheckHandleOrUniqueKeyExistForUpdateIgnoreOrInsertOnDupIgnore(ctx context.C
recordKey := tablecodec.EncodeRecordKey(prefix, recordID)
_, err = txn.Get(ctx, recordKey)
if err == nil {
handleStr := getDuplicateErrorHandleString(t, recordID, data)
handleStr := getDuplicateErrorHandleString(t, recordID, newRow)
return kv.ErrKeyExists.FastGenByArgs(handleStr, "PRIMARY")
} else if !kv.ErrNotExist.Equal(err) {
return err
Expand All @@ -1481,18 +1481,27 @@ func CheckHandleOrUniqueKeyExistForUpdateIgnoreOrInsertOnDupIgnore(ctx context.C

// Check unique key exists.
{
for _, idx := range t.Indices() {
if !IsIndexWritable(idx) {
continue
shouldSkipIgnoreCheck := func(idx table.Index) bool {
if !IsIndexWritable(idx) || !idx.Meta().Unique || (t.Meta().IsCommonHandle && idx.Meta().Primary) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we skip t.Meta().PKIsHandle && idx.Meta().Primary too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think we can skip that because

https://github.com/pingcap/tidb/pull/23894/files#diff-ccb2771ef1931990c0d0ce3703def7dd53bd21d00a8f6b40b59ca0115f776a60R1469-R1480

had check handle, primary key should be same as handle if it's clustered index

Copy link
Contributor Author

@lysu lysu Apr 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

... it seems, there are no indexInfo named Primary when table is PKIsHandle after debug

for example:

create table t4(id int primary key clustered, k int, v int, unique key uk1(k))

t.indices only contain uk1's info, so maybe can avoid this check 😄

return true
}
for _, c := range idx.Meta().Columns {
if modified[c.Offset] {
return false
}
}
if !idx.Meta().Unique {
return true
}

for _, idx := range t.Indices() {
if shouldSkipIgnoreCheck(idx) {
continue
}
vals, err := idx.FetchValues(data, nil)
newVals, err := idx.FetchValues(newRow, nil)
if err != nil {
return err
}
key, _, err := idx.GenIndexKey(sctx.GetSessionVars().StmtCtx, vals, recordID, nil)
key, _, err := idx.GenIndexKey(sctx.GetSessionVars().StmtCtx, newVals, recordID, nil)
if err != nil {
return err
}
Expand All @@ -1503,7 +1512,7 @@ func CheckHandleOrUniqueKeyExistForUpdateIgnoreOrInsertOnDupIgnore(ctx context.C
if err != nil {
return err
}
entryKey, err := genIndexKeyStr(vals)
entryKey, err := genIndexKeyStr(newVals)
if err != nil {
return err
}
Expand Down