Skip to content

ddl: disallow dropping index on auto_increment column#11360

Merged
ngaut merged 6 commits into
pingcap:masterfrom
tangenta:drop-autoinc-index
Jul 23, 2019
Merged

ddl: disallow dropping index on auto_increment column#11360
ngaut merged 6 commits into
pingcap:masterfrom
tangenta:drop-autoinc-index

Conversation

@tangenta
Copy link
Copy Markdown
Contributor

@tangenta tangenta commented Jul 22, 2019

What problem does this PR solve?

drop table if exists t;
create table t (a int auto_increment, unique key (a));
alter table t drop index a;

TiDB:

Query OK, 0 rows affected
Time: 0.003s

Query OK, 0 rows affected
Time: 0.013s

Query OK, 0 rows affected
Time: 0.025s

MySQL 5.7:

Query OK, 0 rows affected
Time: 0.004s

Query OK, 0 rows affected
Time: 0.248s

(1075, 'Incorrect table definition; there can be only one auto column and it must be defined as a key')

What is changed and how it works?

When DropIndex(), check AutoIncrement flag in indexed columns.

Check List

Tests

  • Unit test
  • Integration test

Related changes

  • Need to cherry-pick to the release branch

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 22, 2019

Codecov Report

Merging #11360 into master will decrease coverage by 0.2636%.
The diff coverage is 100%.

@@               Coverage Diff                @@
##             master     #11360        +/-   ##
================================================
- Coverage   81.5135%   81.2498%   -0.2637%     
================================================
  Files           423        423                
  Lines         91045      90170       -875     
================================================
- Hits          74214      73263       -951     
- Misses        11541      11597        +56     
- Partials       5290       5310        +20

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 22, 2019

Codecov Report

Merging #11360 into master will decrease coverage by 0.0991%.
The diff coverage is 100%.

@@               Coverage Diff                @@
##             master     #11360        +/-   ##
================================================
- Coverage   81.3368%   81.2376%   -0.0992%     
================================================
  Files           423        423                
  Lines         90392      90170       -222     
================================================
- Hits          73522      73252       -270     
- Misses        11577      11605        +28     
- Partials       5293       5313        +20

Comment thread planner/core/preprocess.go Outdated
Copy link
Copy Markdown
Contributor

@zimulala zimulala left a comment

Choose a reason for hiding this comment

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

LGTM

@tangenta tangenta added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 22, 2019
Copy link
Copy Markdown
Contributor

@winkyao winkyao left a comment

Choose a reason for hiding this comment

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

LGTM

@tangenta tangenta added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 23, 2019
Comment thread ddl/ddl_api.go Outdated
Comment thread ddl/ddl_api.go Outdated
@ngaut
Copy link
Copy Markdown
Member

ngaut commented Jul 23, 2019

LGTM

@ngaut
Copy link
Copy Markdown
Member

ngaut commented Jul 23, 2019

/run-all-tests

@ngaut ngaut added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Jul 23, 2019
@ngaut ngaut merged commit 435a912 into pingcap:master Jul 23, 2019
@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Jul 23, 2019

cherry pick to release-3.0 failed

sre-bot pushed a commit that referenced this pull request Jul 27, 2019
@you06 you06 added the sig/sql-infra SIG: SQL Infra label Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sig/sql-infra SIG: SQL Infra status/LGT3 The PR has already had 3 LGTM.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants