Allow semi-colon at the end of UNCACHE statement #1320
Conversation
src/parser/mod.rs
Outdated
| } else { | ||
| self.expected("an `EOF`", self.peek_token()) | ||
| }), | ||
| token_loc => self.expected("`EOF` or `;`", token_loc), | ||
| } | ||
| } else { | ||
| self.expected("a `TABLE` keyword", self.peek_token()) |
There was a problem hiding this comment.
(not necessarily related to this PR) looks like we can avoid the has_table condition altogether by using self.expect_keyword instead of self.parse_keyword to parse the TABLE keyword? if so could we make that change to simplify the function in the same go (assuming its a straightforward change)
There was a problem hiding this comment.
Oh nice catch! I can fix this in this PR if this is straightforward, otherwise I can open up a new issue.
src/parser/mod.rs
Outdated
| match self.peek_token() { | ||
| TokenWithLocation { | ||
| token: Token::EOF | Token::SemiColon, | ||
| .. |
There was a problem hiding this comment.
hmm I'm wondering if we don't do an explicitly check for a terminal token after all? looking at the other stmt parse functions seem that's how they handle it, by only parsing the components they need and exiting
There was a problem hiding this comment.
Yeah that's true, but for some reason this parse function only accepts EOF as and of statement. I'll see if we can remove this check.
|
I did what you suggested and it worked. The function also looks much nicer know. Thanks for the help! |
|
Looks like this PR now has a conflict |
Pull Request Test Coverage Report for Build 9611551238Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
8303f1c to
f16c1af
Compare
|
I did a dumb thing, it's fixed now i think. |
Pull Request Test Coverage Report for Build 9626589588Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9626589594Details
💛 - Coveralls |
There was a problem hiding this comment.
Thank you @LorrensP-2158466 -- it is great to see a bug fixed by simplifying the code
Thank you @iffyio for nthe review
| self.expected("a `TABLE` keyword", self.peek_token()) | ||
| } | ||
| self.expect_keyword(Keyword::TABLE)?; | ||
| let if_exists = self.parse_keywords(&[Keyword::IF, Keyword::EXISTS]); |
Closes #1244.
The function that parses UNCACHE statements now also accepts a semi-colon as the end of the statement.
Had to modify one test because the error message
expected ...now also contains the semi-colon.