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 alter table .. truncate partition #8624

Merged
merged 12 commits into from
Dec 11, 2018

Conversation

tiancaiamao
Copy link
Contributor

@tiancaiamao tiancaiamao commented Dec 8, 2018

What problem does this PR solve?

Fix #8435

What is changed and how it works?

Support alter table truncate partition

Check List

Tests

  • Unit test

@ciscoxll @winkyao @zimulala


This change is Reviewable

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@iamxy
Copy link
Member

iamxy commented Dec 8, 2018

/run-sqllogic-test tidb-test=pr/681

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

ddl/partition.go Outdated
// TODO: MySQL behavior for hash partition is weird, "create table .. partition by hash partition 4",
// it use p0, p1, p2, p3 as partition names automatically.
for _, def := range meta.Partition.Definitions {
if strings.EqualFold(def.Name.L, strings.ToLower(parName)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

save strings.ToLower(partName) to local variable? ditto in line#399

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

ddl/partition.go Show resolved Hide resolved
ddl/db_test.go Show resolved Hide resolved
ddl/ddl_api.go Outdated
TableID: meta.ID,
Type: model.ActionTruncateTablePartition,
BinlogInfo: &model.HistoryInfo{},
Args: []interface{}{spec.Name},
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use partition id to remove. Before this job is handling, maybe the spec.Name partition is removed and added back with different partition id?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea !
DropTablePartition also use name rather than partition id, we should give a second thoughts @winkyao @ciscoxll

@zhouqiang-cl
Copy link
Contributor

/run-all-tests

@zhouqiang-cl
Copy link
Contributor

/rebuild

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/rebuild

@tiancaiamao
Copy link
Contributor Author

/run-circleci

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

Copy link
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

@tiancaiamao
Copy link
Contributor Author

PTAL @crazycs520 @ciscoxll @zimulala

Copy link
Contributor

@crazycs520 crazycs520 left a comment

Choose a reason for hiding this comment

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

LGTM

@crazycs520 crazycs520 added the status/LGT2 Indicates that a PR has LGTM 2. label Dec 11, 2018
Copy link
Contributor

@ciscoxll ciscoxll left a comment

Choose a reason for hiding this comment

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

LGTM

@ciscoxll ciscoxll added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Dec 11, 2018
@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/run-unit-test

@tiancaiamao
Copy link
Contributor Author

/run-common-test

@jackysp
Copy link
Member

jackysp commented Dec 11, 2018

/run-common-test tidb-test=pr/688

1 similar comment
@tiancaiamao
Copy link
Contributor Author

/run-common-test tidb-test=pr/688

@tiancaiamao tiancaiamao merged commit e03ef60 into pingcap:master Dec 11, 2018
@tiancaiamao tiancaiamao deleted the truncate-partition branch December 11, 2018 05:53
@tiancaiamao tiancaiamao mentioned this pull request Dec 11, 2018
12 tasks
iamzhoug37 pushed a commit to iamzhoug37/tidb that referenced this pull request Dec 13, 2018
tiancaiamao added a commit to tiancaiamao/tidb that referenced this pull request Jan 16, 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.

support alter table truncate partition for partition table.
8 participants