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

executor: drop partition information in 'show create table' #7388

Merged
merged 2 commits into from
Aug 17, 2018

Conversation

tiancaiamao
Copy link
Contributor

What problem does this PR solve?

Fix show create table in release 2.0

Before this change, create a table like this:

CREATE TABLE t (a int) PARTITION BY RANGE(a) (
 	PARTITION p0 VALUES LESS THAN (10),
 	PARTITION p1 VALUES LESS THAN (20),
 	PARTITION p2 VALUES LESS THAN (MAXVALUE))

show create table results in:

CREATE TABLE t (a int) PARTITION BY RANGE(a) (
 	PARTITION p0 VALUES LESS THAN 10,
 	PARTITION p1 VALUES LESS THAN 20,
 	PARTITION p2 VALUES LESS THAN (MAXVALUE))

It's a illegal SQL and would make syncer broken.

What is changed and how it works?

#6630 fix it in the master branch, but it's hard to cherry-pick.
There is a related fix in show create table like #6332
And also this one #7379
And also some parser related changes for the cherry-pick.

In one word, it's fixed so many times, but never got right.

In this PR, I just drop the partition information, no code, not bug.

Check List

Tests

  • Unit test

Side effects

show create table in release 2.0 branch will not display table partition any more.

@shenli @zhexuany @winkyao

executor/show.go Outdated
}
}
buf.WriteString(")")
// Partition info is truncated in release-2.0 branch.
Copy link
Member

Choose a reason for hiding this comment

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

Please update the comment in line 592 and remove the following lines.

@tiancaiamao
Copy link
Contributor Author

PTAL @shenli

Copy link
Member

@shenli shenli left a comment

Choose a reason for hiding this comment

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

LGTM

@shenli shenli added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 15, 2018
@tiancaiamao
Copy link
Contributor Author

PTAL @winkyao @zhexuany

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

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/run-all-tests tikv=release-2.0 pd=release-2.0 tidb-test=release-2.0

@tiancaiamao
Copy link
Contributor Author

@shenli

@tiancaiamao
Copy link
Contributor Author

@shenli Ping

@zimulala zimulala added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 16, 2018
@coocood coocood merged commit 1fb887f into pingcap:release-2.0 Aug 17, 2018
@tiancaiamao tiancaiamao added this to In Progress in Table Partitioning via automation Aug 26, 2018
@tiancaiamao tiancaiamao moved this from In Progress to Done in Table Partitioning Aug 26, 2018
@tiancaiamao tiancaiamao deleted the show-create-table branch November 21, 2018 13:51
@zz-jason zz-jason moved this from Issue: Done to PR: Donw in Table Partitioning Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/execution SIG execution status/LGT2 Indicates that a PR has LGTM 2.
Projects
No open projects
Table Partitioning
  
PR: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants