Skip to content

DDL parsing improvements - #65

Merged
nickolay merged 5 commits into
apache:masterfrom
nickolay:pr/ddl-improvements
Jun 2, 2019
Merged

nickolay merged 5 commits into
apache:masterfrom
nickolay:pr/ddl-improvements

Conversation

@nickolay

Copy link
Copy Markdown
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

coveralls commented May 11, 2019 •

Copy link
Copy Markdown

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

nickolay commented Jun 2, 2019

Copy link
Copy Markdown
Contributor Author

@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

benesch commented Jun 2, 2019

Copy link
Copy Markdown
Contributor

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

nickolay commented Jun 2, 2019

Copy link
Copy Markdown
Contributor Author

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