*: support adding/dropping the primary key by a configuration #13229
Conversation
|
PTAL @crazycs520 @bb7133 @jackysp |
| @@ -68,6 +82,205 @@ func (s *testSerialSuite) TearDownSuite(c *C) { | |||
| } | |||
| } | |||
|
|
|||
| func (s *testSerialSuite) TestPrimaryKey(c *C) { | |||
There was a problem hiding this comment.
The new tests for this file come from db_integration_test.go because they require newCfg.EnablePKIsHandle = true.
Codecov Report
@@ Coverage Diff @@
## master #13229 +/- ##
=========================================
Coverage 80.504% 80.504%
=========================================
Files 472 472
Lines 115711 115711
=========================================
Hits 93152 93152
Misses 15382 15382
Partials 7177 7177 |
ghost
requested review from
wshwsh12 and
XuHuaiyu
and removed request for
a team
config/config.toml.example
Outdated
| @@ -66,6 +66,11 @@ delay-clean-table-lock = 0 | |||
| # Maximum number of the splitting region, which is used by the split region statement. | |||
| split-region-max-num = 1000 | |||
|
|
|||
| # enable-pk-is-handle is used to control alter primary key feature. Default is true, indicate the alter primary key feature is disabled. | |||
There was a problem hiding this comment.
It looks really confused here. Why true means disable since the name is enableXXX. And the name makes me wondering enable pk to do what.
There was a problem hiding this comment.
Do you have any advice?
There was a problem hiding this comment.
How about change-primary-key?
There may be a potential problem when each TiDB configuration is not the same. But considering that this may be a temporary solution, it should be fine.
There was a problem hiding this comment.
Mainly, if this configuration allows, but the table's pkIsHandle is true, we also do not support drop the primary key. I think it still a little different. @jackysp @lamxTyler
There was a problem hiding this comment.
Mainly, if this configuration allows, but the table's pkIsHandle is true, we also do not support drop the primary key. I think it still a little different. @jackysp @lamxTyler
There was a problem hiding this comment.
I think this difference can be explained in comments and documentation. pk-is-handle is not easy for users to understand the meaning.
There was a problem hiding this comment.
OK. I change it to enable-alter-primary-key. @jackysp @lamxTyler
There was a problem hiding this comment.
After discussing with PM, I change it to alter-primary-key.
zimulala
added
needs-cherry-pick-3.1
status/can-merge
|
/run-all-tests |
|
@zimulala merge failed. |
|
/run-all-tests |
|
/run-common-test tidb-test=pr/948 |
|
/merge |
|
/run-all-tests |
|
@zimulala merge failed. |
|
/run-common-test tidb-test=pr/948 |
1 similar comment
|
/run-common-test tidb-test=pr/948 |
|
cherry pick to release-3.0 failed |
|
cherry pick to release-3.1 failed |
What problem does this PR solve?
Support adding/dropping the primary key by a configuration.
What is changed and how it works?
Using enable-pk-is-handle to control alter primary key feature. The default is true, which means that the alter primary key feature is disabled.
If we set this configuration item to false, we can add a primary key through "alter table", but we may not be able to drop the primary key. In order to support the "drop primary key" operation, this configuration must be false, and the table does not have the pkIsHandle flag.
Its implementation is similar to the "add/drop index" except that "add primary key" requires an additional check for not null.
Related PR pingcap/parser#617
Check List
Tests
Related changes
Release note
enable-pk-is-handle.