executor: skip ignore check for not update indexes #23894
Conversation
table/tables/tables.go
Outdated
| @@ -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, oldRow, newRow []types.Datum) error { | |||
There was a problem hiding this comment.
I think you could use modified in updateRecord to avoid to compare once again.
|
/run-all-tests |
table/tables/tables.go
Outdated
| } | ||
| for _, c := range idx.Meta().Columns { | ||
| if modified[c.Offset] { | ||
| return true |
There was a problem hiding this comment.
A bit confused, if this index related column value is changed, should we do the check?
There was a problem hiding this comment.
😵 my fault when extract method..fixed...it only need check when value changed
|
/run-all-tests |
|
/lgtm |
| if !IsIndexWritable(idx) { | ||
| continue | ||
| canSkipIgnoreCheck := func(idx table.Index) bool { | ||
| if !IsIndexWritable(idx) || !idx.Meta().Unique || (t.Meta().IsCommonHandle && idx.Meta().Primary) { |
There was a problem hiding this comment.
Should we skip t.Meta().PKIsHandle && idx.Meta().Primary too?
There was a problem hiding this comment.
i think we can skip that because
had check handle, primary key should be same as handle if it's clustered index
There was a problem hiding this comment.
... 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 😄
|
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by writing |
|
CI failed. |
|
/merge |
|
This pull request has been accepted and is ready to merge. Commit hash: 02c28cb
|
|
/run-all-tests |
|
cherry pick to release-5.0 in PR #23976 |
What problem does this PR solve?
Issue Number: close #23892
Problem Summary:
for
insert ignore on duplicate updatestmt, "ignore check" should only check the index that real changeonly new row value meet duplicated in indexes need be ignore, and it can continue update if new value == old value, due to old row/indexes will be removed before upsert
What is changed and how it works?
What's Changed, How it Works:
only ignore check for real changed indexes
Related changes
Check List
Tests
Side effects
Release note
This change is