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

Jump to bottom

ddl: add alter index visibility DDL support #16914

Merged
merged 18 commits into from
May 8, 2020
Merged

ddl: add alter index visibility DDL support #16914

merged 18 commits into from
May 8, 2020

Conversation

Deardrops
Copy link
Contributor

@Deardrops Deardrops commented Apr 28, 2020 •
edited
Loading

What problem does this PR solve?

Issue Number: close #9246

Problem Summary: Support Invisible Indexes

What is changed and how it works?

Proposal: #15338

What's Changed:

This PR add new DDL statement support:

alter table t alter index idx invisible;
alter table t alter index idx visible;

How it Works:

Add a new DDL job AlterTableIndexInvisible.

Related changes

  • None

Check List

Tests

  • Integration test

Side effects

  • Performance regression
    • Consumes more CPU
    • Consumes more MEM
  • Breaking backward compatibility

Release note

  • Support invisible indexes

@github-actions github-actions bot added the sig/sql-infra SIG: SQL Infra label Apr 28, 2020
@Deardrops Deardrops changed the title ddl: add alter index visibility statement ddl: add alter index visibility ddl support Apr 28, 2020
@Deardrops Deardrops changed the title ddl: add alter index visibility ddl support ddl: add alter index visibility DDL support Apr 29, 2020
@Deardrops Deardrops mentioned this pull request Apr 29, 2020
Merged
zimulala
zimulala reviewed Apr 30, 2020
View reviewed changes
Copy link
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.

Could we need to update the result of show create table?

ddl/ddl_api.go Show resolved Hide resolved
ddl/index.go Outdated Show resolved Hide resolved
ddl/db_integration_test.go Outdated Show resolved Hide resolved
@Deardrops
Copy link
Contributor Author

Deardrops commented Apr 30, 2020 •
edited
Loading

Could we need to update the result of show create table?

@zimulala This job has finished in #15366

zimulala
zimulala reviewed Apr 30, 2020
View reviewed changes
ddl/rollingback.go Show resolved Hide resolved
ddl/index.go Outdated Show resolved Hide resolved
@Deardrops Deardrops requested a review from zimulala April 30, 2020 07:08
djshow832
djshow832 reviewed Apr 30, 2020
View reviewed changes
ddl/db_integration_test.go Outdated Show resolved Hide resolved
ddl/db_integration_test.go Show resolved Hide resolved
@sre-bot
Copy link
Contributor

sre-bot commented May 2, 2020

@zimulala, @djshow832, PTAL.

1 similar comment
@sre-bot
Copy link
Contributor

sre-bot commented May 4, 2020

@zimulala, @djshow832, PTAL.

@Deardrops Deardrops requested a review from djshow832 May 6, 2020 07:04
@Deardrops
Copy link
Contributor Author

@djshow832 @zimulala All comment addressed, PTAL

@Deardrops Deardrops requested a review from a team as a code owner May 7, 2020 04:58
@ghost ghost requested a review from lzmhhh123 May 7, 2020 04:58
AilinKid
AilinKid reviewed May 7, 2020
View reviewed changes
Copy link
Contributor

@AilinKid AilinKid left a comment

Choose a reason for hiding this comment

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

Rest LGTM

ddl/ddl_api.go Show resolved Hide resolved
bindinfo/bind_test.go Outdated Show resolved Hide resolved
bindinfo/bind_test.go Outdated Show resolved Hide resolved
AilinKid
AilinKid reviewed May 7, 2020
View reviewed changes
ddl/index.go Show resolved Hide resolved
@codecov
Copy link

codecov bot commented May 8, 2020 •
edited
Loading

Codecov Report

Merging #16914 into master will decrease coverage by 0.0564%.
The diff coverage is 70.3125%.

@@               Coverage Diff                @@
##             master     #16914        +/-   ##
================================================
- Coverage   80.0322%   79.9758%   -0.0565%     
================================================
  Files           510        510                
  Lines        139049     138857       -192     
================================================
- Hits         111284     111052       -232     
- Misses        18798      18829        +31     
- Partials       8967       8976         +9

AilinKid
AilinKid approved these changes May 8, 2020
View reviewed changes
Copy link
Contributor

@AilinKid AilinKid left a comment

Choose a reason for hiding this comment

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

LGTM

@AilinKid
Copy link
Contributor

AilinKid commented May 8, 2020

/run-all-tests

@Deardrops
Copy link
Contributor Author

/run-unit-test

@Deardrops Deardrops added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels May 8, 2020
wjhuang2016
wjhuang2016 approved these changes May 8, 2020
View reviewed changes
Copy link
Member

@wjhuang2016 wjhuang2016 left a comment

Choose a reason for hiding this comment

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

LGTM

@Deardrops Deardrops mentioned this pull request May 8, 2020
Merged
5 tasks
@Deardrops
Copy link
Contributor Author

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label May 8, 2020
@sre-bot
Copy link
Contributor

sre-bot commented May 8, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented May 8, 2020

@Deardrops merge failed.

@Deardrops
Copy link
Contributor Author

/merge

@sre-bot
Copy link
Contributor

sre-bot commented May 8, 2020

/run-all-tests

@sre-bot sre-bot merged commit 65b7f0f into pingcap:master May 8, 2020
Deardrops added a commit to Deardrops/tidb that referenced this pull request May 9, 2020
@Deardrops
ddl: add alter index visibility DDL support (pingcap#16914)
01cbe85
This was referenced May 9, 2020
Merged
Merged
@zimulala zimulala mentioned this pull request May 22, 2020
Merged
5 tasks
This was referenced Oct 16, 2020
Merged
Closed
@ti-srebot ti-srebot mentioned this pull request Oct 16, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djshow832 djshow832 djshow832 left review comments

@zimulala zimulala zimulala left review comments

@wjhuang2016 wjhuang2016 wjhuang2016 approved these changes

@AilinKid AilinKid AilinKid approved these changes

@lzmhhh123 lzmhhh123 Awaiting requested review from lzmhhh123

Assignees
No one assigned
Labels
sig/planner SIG: Planner sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer. status/LGT3 The PR has already had 3 LGTM.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

UCP: Support Invisible Indexes
6 participants
@Deardrops @sre-bot @AilinKid @zimulala @wjhuang2016 @djshow832