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

parser: support some new syntax #392

Merged
merged 15 commits into from
Jul 22, 2019
Merged

Conversation

zier-one
Copy link
Contributor

@zier-one zier-one commented Jul 17, 2019

support followed syntax:

ALTER TABLE t LOCK [=] lock_type

ALTER TABLE t ALGORITHM [=] algorithm_type

ALTER TABLE t ALTER COLUMN a SET DEFAULT (expr)

ALTER TABLE t ADD CONSTRAINT aaa CHECK (expr) [NOT [ENFORCED]]
CREATE TABLE t (a INT CHECK(expr) [NOT [ENFORCED]])
CREATE TABLE t (a INT CHECK(expr) NOT NULL)
(CHECK only parsed but not implement function)
Here I refer to the implementation of MySQL:
https://github.com/mysql/mysql-server/blob/c000dcf1a9b2ee4a4f986dde08c1f6e1b6461a18/sql/sql_yacc.yy#L6733

BTW: Thanks for the guidance from @kennytm

What problem does this PR solve?

Improve the compatibility of tidb parser

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
    i tested SET DEFAULT (expr) in tidb which use the parser of this commi.
    we support const value like : (1+1),(1*1). but we don't support functionexpr like: (now())
  • No code

Code changes

  • Has exported function/method change
    none
  • Has exported variable/fields change
    add a new lex token named 'enforced'
  • Has interface methods change
    none

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation
  • Need to be included in the release note

@zier-one
Copy link
Contributor Author

/run-all-tests

@codecov
Copy link

codecov bot commented Jul 17, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@ded7e04). Click here to learn what that means.
The diff coverage is 63.63%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master    #392   +/-   ##
========================================
  Coverage          ?   70.1%           
========================================
  Files             ?      32           
  Lines             ?    7405           
  Branches          ?       0           
========================================
  Hits              ?    5191           
  Misses            ?    1701           
  Partials          ?     513
Impacted Files Coverage Δ
parser.go 70.58% <ø> (ø)
misc.go 96.42% <ø> (ø)
yy_parser.go 83.33% <100%> (ø)
ast/ddl.go 77.56% <55.55%> (ø)

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 ded7e04...e6068bc. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 17, 2019

Codecov Report

Merging #392 into master will increase coverage by 0.05%.
The diff coverage is 77.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #392      +/-   ##
==========================================
+ Coverage    70.1%   70.16%   +0.05%     
==========================================
  Files          32       32              
  Lines        7399     7433      +34     
==========================================
+ Hits         5187     5215      +28     
- Misses       1700     1703       +3     
- Partials      512      515       +3
Impacted Files Coverage Δ
parser.go 70.58% <ø> (ø) ⬆️
misc.go 96.42% <ø> (ø) ⬆️
yy_parser.go 83.33% <100%> (+0.26%) ⬆️
ast/ddl.go 77.73% <76.47%> (+0.08%) ⬆️

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 191583a...a90291a. Read the comment docs.

@zier-one zier-one marked this pull request as ready for review July 17, 2019 09:00
@zier-one
Copy link
Contributor Author

@kennytm @tiancaiamao PTAL

parser.y Outdated Show resolved Hide resolved
parser.y Outdated Show resolved Hide resolved
parser.y Show resolved Hide resolved
@zier-one
Copy link
Contributor Author

@imtbkcat PTAL

@tiancaiamao
Copy link
Collaborator

integration-test fail

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.

Some code is spaced badly. Rest LGTM.

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

@kennytm @tiancaiamao PTAL again

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

@kennytm kennytm added the status/LGT1 LGT1 label Jul 19, 2019
@tiancaiamao
Copy link
Collaborator

LGTM

@tiancaiamao tiancaiamao added status/LGT2 LGT2 and removed status/LGT1 LGT1 labels Jul 22, 2019
tiancaiamao
tiancaiamao previously approved these changes Jul 22, 2019
@zier-one zier-one merged commit 837bc29 into pingcap:master Jul 22, 2019
@zier-one zier-one deleted the fix_alter_table_1 branch July 22, 2019 02:29
tangenta pushed a commit to tangenta/parser that referenced this pull request Nov 18, 2019
tangenta pushed a commit to tangenta/parser that referenced this pull request Nov 20, 2019
tiancaiamao pushed a commit to tiancaiamao/parser that referenced this pull request Apr 27, 2021
lyonzhi pushed a commit to lyonzhi/parser that referenced this pull request Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants