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.

Sign up for GitHub

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

Jump to bottom

parser: fix remove partitioning syntax #396

Merged
merged 6 commits into from
Jul 26, 2019
Merged

parser: fix remove partitioning syntax #396

merged 6 commits into from
Jul 26, 2019

Conversation

tangenta
Copy link
Contributor

@tangenta tangenta commented Jul 24, 2019 •
edited by zier-one
Loading

What problem does this PR solve?

Fix compatibility problem about keyword REMOVE PARTITIONING

Issue: #402

MySQL Syntax:

alter_specification:
...
  | REMOVE PARTITIONING

Bad SQL Case:

alter table t remove partitioning;
alter table t lock = default remove partitioning;

Check List

Tests

  • Unit test

@tangenta tangenta changed the title parser: add remove partition syntax [DNM] parser: add remove partition syntax Jul 24, 2019
@zier-one zier-one changed the title [DNM] parser: add remove partition syntax parser: fix remove partition syntax Jul 24, 2019
@zier-one zier-one self-requested a review July 24, 2019 06:48
@zier-one zier-one added the DNM label Jul 24, 2019
@zier-one zier-one changed the title parser: fix remove partition syntax parser: fix remove partitioning syntax Jul 24, 2019
@codecov
Copy link

codecov bot commented Jul 24, 2019 •
edited
Loading

Codecov Report

Merging #396 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #396      +/-   ##
==========================================
+ Coverage   70.22%   70.22%   +<.01%     
==========================================
  Files          32       32              
  Lines        7448     7450       +2     
==========================================
+ Hits         5230     5232       +2     
  Misses       1703     1703              
  Partials      515      515
Impacted Files Coverage Δ
parser.go 70.58% <ø> (ø) ⬆️
misc.go 96.42% <ø> (ø) ⬆️
ast/ddl.go 77.94% <100%> (+0.03%) ⬆️
ast/functions.go 74.89% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca2f45b...1f6636b. Read the comment docs.

kennytm
kennytm requested changes Jul 24, 2019
View reviewed changes
Copy link
Contributor

@kennytm kennytm left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The "REMOVE PARTITIONING" should be one of AlterTableSpec.

zier-one
zier-one reviewed Jul 24, 2019
View reviewed changes
parser.y Outdated Show resolved Hide resolved
kennytm
kennytm reviewed Jul 24, 2019
View reviewed changes
parser.y Outdated Show resolved Hide resolved
zier-one
zier-one reviewed Jul 24, 2019
View reviewed changes
parser.y Outdated Show resolved Hide resolved
parser.y Outdated Show resolved Hide resolved
@zier-one
Copy link
Contributor

LGTM
@kennytm PTAL

@zier-one zier-one added the status/LGT1 LGT1 label Jul 25, 2019
kennytm
kennytm reviewed Jul 25, 2019
View reviewed changes
ast/ddl.go Outdated Show resolved Hide resolved
zier-one
zier-one reviewed Jul 26, 2019
View reviewed changes
parser.y Outdated Show resolved Hide resolved
parser.y Outdated Show resolved Hide resolved
parser.y Outdated Show resolved Hide resolved
parser.y Outdated Show resolved Hide resolved
zier-one
zier-one reviewed Jul 26, 2019
View reviewed changes
parser.y Outdated Show resolved Hide resolved
zier-one
zier-one reviewed Jul 26, 2019
View reviewed changes
Copy link
Contributor

@zier-one zier-one left a comment

Choose a reason for hiding this comment

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

Rest LGTM

kennytm
kennytm reviewed Jul 26, 2019
View reviewed changes
Copy link
Contributor

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

LGTM after the typographic comments are fixed.

Is this PR still "DNM"?

@zier-one zier-one removed the DNM label Jul 26, 2019
@kennytm kennytm merged commit 53c769c into pingcap:master Jul 26, 2019
@zier-one zier-one added status/LGT2 LGT2 and removed status/LGT1 LGT1 labels Jul 29, 2019
@zier-one zier-one mentioned this pull request Jul 29, 2019
Closed
58 tasks
tiancaiamao pushed a commit to tiancaiamao/parser that referenced this pull request Apr 27, 2021
@tangenta @kennytm
parser: fix remove partitioning syntax (pingcap#396)
b2776f8 
* add support for REMOVE PARTITIONING syntax

* remove unused value

* add type for AlterTableSpec

* indent with tab

* indent with tab
lyonzhi pushed a commit to lyonzhi/parser that referenced this pull request Apr 25, 2024
@tangenta @kennytm
parser: fix remove partitioning syntax (pingcap#396)
789ebcd 
* add support for REMOVE PARTITIONING syntax

* remove unused value

* add type for AlterTableSpec

* indent with tab

* indent with tab
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zier-one zier-one zier-one left review comments

@kennytm kennytm kennytm approved these changes

Assignees
No one assigned
Labels
status/LGT2 LGT2 type/compatibility
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tangenta @zier-one @kennytm