-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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.
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: support drop indexes for multi-schema change #35883
Conversation
[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 submitting an approval review. |
Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/b062089fcf5ccf58dedc16a05de886942b0294da |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/merge |
This pull request has been accepted and is ready to merge. Commit hash: a31ed2a
|
// the second args will be used in onDropIndex. | ||
job.Args = []interface{}{indexInfo.Name, getPartitionIDs(tblInfo)} | ||
// the second and the third args will be used in onDropIndex. | ||
job.Args = []interface{}{indexInfo.Name, false /* ifExists */, getPartitionIDs(tblInfo)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this break the compatibility, if it is inserted into the middle, @tangenta ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For DropColumn()
, the exists
argument is the last one... there are two cases:
- New version TiDB creates a job (args: [idx_name, ifExists]), old version TiDB runs the job(ifExists is ignored).
- Old version TiDB creates a job(args: [idx_name]), new version TiDB runs the job(ifExists is decoded to
false
).
In the worse case, the DDL owner TiDB reports an error. The data is consistent.
TiDB MergeCI notify🔴 Bad News! [2] CI still failing after this pr merged.
|
* upstream/master: (77 commits) infoschema: add a simply store for DM's SchemaTracker (pingcap#35954) planner: refactor "access object" related logic and prepare for binary plan (pingcap#35890) telemetry: Initialize telemetry asynchronously (pingcap#35898) sessionctx/variable: convert TiDBLogFileMaxDays and TiDBConfig to ScopeInstance (pingcap#35733) session, sessionctx/variable: ensure sysvars are always in lower case (pingcap#35659) *: add telemetry support for multi-schema change (pingcap#35977) topsql: compress plan instead of dropping it (pingcap#35973) *: use pdqsort to improve sort performance (pingcap#34404) ddl: refine GetDDLInfo function (pingcap#35969) *: Make the code cleaner in session.ExecutePreparedStmt (pingcap#35943) ddl: remove `onDropIndexes` (pingcap#35960) ddl: refine history job code (pingcap#35967) sessionctx,kv,planner: add system variable for fine_grained_shuffle (pingcap#35256) *: change remote cache (pingcap#35968) *: enable gosec (pingcap#35873) plancodec: fix a miss case in conversion (pingcap#35957) ddl: support drop indexes for multi-schema change (pingcap#35883) pdutil: fix unstable test TestPauseSchedulersByKeyRange (pingcap#35949) br: optimize communication with tikv stores. pingcap#31722 (pingcap#35147) txn: rename snapshot related methods in txnManager. (pingcap#35912) ...
What problem does this PR solve?
Issue Number: ref #14766
Problem Summary:
This PR supports drop indexes for multi-schema change.
What is changed and how it works?
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.