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

*: enable range typed table partition #8011

Merged
merged 4 commits into from
Oct 26, 2018

Conversation

tiancaiamao
Copy link
Contributor

What problem does this PR solve?

Enable range typed table partition by default.

What is changed and how it works?

If the SQL is create table ... partition by range ...,
the PartitionInfo.Enable field will be set to true
So this kind of table partition is handled.

Check List

Tests

  • No code

Side effects

  • Breaking backward compatibility

If a user create a range typed table partition with this TiDB binary, he can't degrade to an old version.

@shenli @ciscoxll

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@tiancaiamao tiancaiamao mentioned this pull request Oct 23, 2018
12 tasks
@tiancaiamao tiancaiamao added the type/enhancement The issue or PR belongs to an enhancement. label Oct 23, 2018
@iamxy
Copy link
Member

iamxy commented Oct 23, 2018

/run-integration-tests

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

@tiancaiamao tiancaiamao added status/LGT1 Indicates that a PR has LGTM 1. status/all tests passed labels Oct 23, 2018
Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

So there is no way to disable the range partition. I prefer to change the flag of enable-partition true by default.

@tiancaiamao
Copy link
Contributor Author

After a partitioned table is created, turn off the flag is to late:
there is no way to handle a partitioned table as a normal table because the data is already persisted.

So this feature is controlled by the data, rather than by the code, provide a flag can't help.
@jackysp

@jackysp
Copy link
Member

jackysp commented Oct 24, 2018

For some POCs, the customers could rebuild the cluster and continue the POC test if they could disable the partition feature. Otherwise, the test may be blocked. @tiancaiamao

executor/show.go Outdated
@@ -620,6 +620,7 @@ func (e *ShowExec) fetchShowCreateTable() error {
if partitionInfo != nil {
// this if statement takes care of range columns case
if partitionInfo.Columns != nil && partitionInfo.Type == model.PartitionTypeRange {

Copy link
Contributor

Choose a reason for hiding this comment

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

Why add this line?

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

PTAL @jackysp @zimulala

Copy link
Member

@jackysp jackysp 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

Any idea about the error? @zimulala

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

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.

LGTM

@zimulala zimulala added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 26, 2018
@zz-jason zz-jason merged commit 5831de2 into pingcap:master Oct 26, 2018
@tiancaiamao tiancaiamao deleted the enable-table-partition branch August 9, 2019 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants