Skip to content

DDL parsing improvements #65

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

Merged
merged 5 commits into from
Jun 2, 2019
Merged

Conversation

nickolay
Copy link
Contributor

See the individual commits' messages.

I wasn't sure which of unsized types to pick in andygrove@1492f42 -- if there's a reason to pick another one, please let me know.

@nickolay nickolay requested a review from andygrove May 11, 2019 22:13
@coveralls
Copy link

coveralls commented May 11, 2019

Pull Request Test Coverage Report for Build 228

  • 185 of 212 (87.26%) changed or added relevant lines in 7 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.4%) to 90.391%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/sqlast/mod.rs 16 18 88.89%
tests/sqlparser_common.rs 35 37 94.59%
src/sqlast/ddl.rs 41 45 91.11%
src/sqlast/sqltype.rs 2 9 22.22%
src/sqlparser.rs 84 96 87.5%
Totals Coverage Status
Change from base Build 226: 0.4%
Covered Lines: 3302
Relevant Lines: 3653

💛 - Coveralls

nickolay added 5 commits June 2, 2019 13:54
- merge PrimaryKey and UniqueKey variants
- support `CHECK` constraints, removing the separate `Key` struct
- make `CONSTRAINT constraint_name` optional
- remove `KEY` without qualifiers (wasn't parsed and there doesn't
  appear to be such a thing)
- change `UNIQUE KEY` -> `UNIQUE`
- change `REMOVE CONSTRAINT` -> `DROP CONSTRAINT` and note its parsing
  is not implemented

Spec:
- ANSI SQL: see <table constraint definition> in https://jakewheat.github.io/sql-overview/sql-2011-foundation-grammar.html#_11_6_table_constraint_definition
- Postgres: look for "and table_constraint is:" in https://www.postgresql.org/docs/11/sql-altertable.html
Since other ALTER statements will have separate sub-commands.
At least MSSQL supports it, not sure about others.
The tokenizer emits a separate Token for +/- signs, so the value of
Value::Long() (as well as of parse_literal_int()) may never be negative.

Also we have been using both u64 and usize to represent a parsed
unsigned number. Change to using u64 for consistency.
@nickolay nickolay force-pushed the pr/ddl-improvements branch from 1492f42 to d9edc25 Compare June 2, 2019 11:01
@nickolay nickolay requested a review from benesch June 2, 2019 11:02
@nickolay
Copy link
Contributor Author

nickolay commented Jun 2, 2019

@benesch I've rebased this on master, please take a look - is this good to merge or would you rather finish landing your patches?

@benesch
Copy link
Contributor

benesch commented Jun 2, 2019

Boy, I wish I'd looked at this before I implemented all this myself. 🤦‍♂ I think you should merge this, and then I'll fix any mismatches between our two implementations (if there even are any).

@nickolay nickolay merged commit ebb82b8 into apache:master Jun 2, 2019
@nickolay nickolay deleted the pr/ddl-improvements branch June 2, 2019 17:53
@nickolay
Copy link
Contributor Author

nickolay commented Jun 2, 2019

OK, merged. Thanks for the quick reply!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants