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

Support CREATE TABLE x AS TABLE y #704

Merged
merged 14 commits into from Dec 1, 2022
Merged

Conversation

sarahyurick
Copy link
Contributor

@sarahyurick sarahyurick commented Nov 8, 2022

Closes #614

My solution was to parse CREATE TABLE x AS TABLE y the same as CREATE TABLE x AS SELECT * FROM y.

@coveralls
Copy link

coveralls commented Nov 9, 2022

Pull Request Test Coverage Report for Build 3587526224

  • 62 of 72 (86.11%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.001%) to 86.345%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/ast/query.rs 7 10 70.0%
src/parser.rs 26 33 78.79%
Files with Coverage Reduction New Missed Lines %
src/ast/query.rs 1 84.77%
Totals Coverage Status
Change from base Build 3586103034: -0.001%
Covered Lines: 12242
Relevant Lines: 14178

💛 - Coveralls

src/parser.rs Outdated Show resolved Hide resolved
src/ast/query.rs Outdated Show resolved Hide resolved
@sarahyurick
Copy link
Contributor Author

Hi @alamb, let me know if these updates look reasonable 🙂

Copy link
Collaborator

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Sorry for the delay in review @sarahyurick -- this is looking better but not quite ready yet I think

@@ -84,7 +84,7 @@ pub enum SetExpr {
},
Values(Values),
Insert(Statement),
// TODO: ANSI SQL supports `TABLE` here.
Table(Box<Table>),
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

src/ast/query.rs Show resolved Hide resolved
src/parser.rs Outdated Show resolved Hide resolved
tests/sqlparser_common.rs Outdated Show resolved Hide resolved
tests/sqlparser_common.rs Outdated Show resolved Hide resolved
src/parser.rs Outdated Show resolved Hide resolved
src/parser.rs Show resolved Hide resolved
@sarahyurick
Copy link
Contributor Author

Thanks @alamb ! Updated to use peek_token() (and also peek_nth_token()) and does not fail on your whitespace example anymore. I wasn't exactly sure how to use parse_identifier() for this, but hopefully the logic for parse_as_table() should be a lot cleaner now.

Copy link
Collaborator

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks @sarahyurick -- I think this PR now looks good. I have some ideas of how to simplify the code a little more but I'll make a follow on PR with that change.

Thanks again for sticking with this

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.

Support CREATE TABLE x AS TABLE y
3 participants