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.

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 dropping a column with single-value indices #18812

Closed
bb7133 opened this issue Jul 27, 2020 · 5 comments
Closed

ddl: support dropping a column with single-value indices #18812

bb7133 opened this issue Jul 27, 2020 · 5 comments
Assignees
Labels
feature/accepted This feature request is accepted by product managers type/enhancement The issue or PR belongs to an enhancement. type/feature-request Categorizes issue or PR as related to a new feature.

Comments

@bb7133
Copy link
Member

bb7133 commented Jul 27, 2020

Feature Request

Is your feature request related to a problem? Please describe:
Support dropping a column with single-value indices. For now, the ALTER TABLE ... DROP COLUMN statement returns an error when dropping a column that is covered with index:

tidb> create table t(a int, b int, index idx_b(b));
Query OK, 0 rows affected (0.01 sec)

tidb> alter table t drop column b;
ERROR 8200 (HY000): can't drop column b with index covered now

Describe the feature you'd like:
See #3364 for the full feature requests.

Describe alternatives you've considered:
Manually, the ALTER TABLE ... DROP COLUMN in TiDB can be archived by 2 separate steps:

  1. Drop all indices that cover the column to be dropped.
  2. Drop the column without any index.

However, for some third-party frameworks that relies on dropping columns directly, workarounds have to be done for this limitation. This can be disappointing.

Teachability, Documentation, Adoption, Migration Strategy:
Compared with #3364, which is the full support for ALTER TABLE ... DROP COLUMN, this task is relatively easy. We can achieve by combining the current codes of ALTER TABLE ... DROP COLUMN and ALTER TABLE ... DROP INDEX. More help is available by referring:

Here are some major code changes that may be helpful:

  1. Update function isDroppableColumn by removing the restriction of dropping column/columns that are covered by single-value indicies

  2. Update function onDropColumn and onDropColumns(maybe they can/should be combined together): check if they're droppable and retrieve all the single-value indices that cover the columns, and change the state of indicies with the columns(StatePublic -> StateWriteOnly -> StateDeleteOnly -> StateDeleteReorgnization -> StateNone). It would be much better if we can reuse the codes in onDropIndex by some code refactorings.

  3. Update function insertJobIntoDeleteRangeTable to add the range information for dropped columns along with all the indicies.

  4. May need to update ApplyDiff. It is possible that this function works fine without modification because the state of column/index has been updated together with the new TableInfo. However, it is worth checking this function still.

  5. Add unit tests/integration tests. It should be aware that, for a specific column, the related index/indices can be changed when the 'DropTable' DDL job is waiting in the DDL queue and it's worth making some tests for it.

  6. Update rollingbackDropColumn function to process rollback.

@bb7133 bb7133 added the type/feature-request Categorizes issue or PR as related to a new feature. label Jul 27, 2020
@bb7133 bb7133 added the type/enhancement The issue or PR belongs to an enhancement. label Jul 27, 2020
@blacktear23
Copy link
Contributor

@bb7133 there has 6 item, we should edit rollingback.go: rollingbackDropColumn function to process rollback

@scsldb scsldb added the feature/reviewing This feature request is reviewing by product managers label Jul 29, 2020
@zz-jason zz-jason added feature/accepted This feature request is accepted by product managers and removed feature/reviewing This feature request is reviewing by product managers labels Aug 14, 2020
@bb7133
Copy link
Member Author

bb7133 commented Aug 14, 2020

@bb7133 there has 6 item, we should edit rollingback.go: rollingbackDropColumn function to process rollback

Thanks, updated.

I will close this feature since #18852 is merged.

@bb7133 bb7133 closed this as completed Aug 14, 2020
@zz-jason
Copy link
Member

can we backport it to release 4.0?

@bb7133
Copy link
Member Author

bb7133 commented Aug 14, 2020

can we backport it to release 4.0?

Unless it is critical for some users, it's better to keep it in V5.0 since this feature is just a part of the multiple schema changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/accepted This feature request is accepted by product managers type/enhancement The issue or PR belongs to an enhancement. type/feature-request Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

6 participants