Enable parser to parse create external clauses in arbitrary order #6257
Conversation
github-actions
bot
added
core
| ensure_not_set(&builder.options, "OPTIONS")?; | ||
| builder.options = Some(self.parse_options()?); | ||
| } else { | ||
| break; |
There was a problem hiding this comment.
Not sure will it be worth adding more checks like expect_one_of_keywords (or other smarter ways) to make the user experience better? (though I couldn't find it's universally applied to all parser paths)
e.g. a typo like
CREATE EXTERNAL TABLE t(c1 int)
STORED AS CSV
WITH HEAD ROW
LOCATION 'foo.csv';will turn into error sql parser error: Missing LOCATION clause in CREATE EXTERNAL TABLE statement which is a bit confusing.
There was a problem hiding this comment.
I agree this would make a better user experience and I think it would be a good follow on project. I will file the ticket
| } else { | ||
| vec![] | ||
| }; | ||
| #[derive(Default)] |
| ############### | ||
|
|
||
| # Missing `STORED AS` and `LOCATION` clauses | ||
| statement error DataFusion error: SQL error: ParserError\("Missing STORED AS clause in CREATE EXTERNAL TABLE statement"\) |
There was a problem hiding this comment.
👍 these tests are so nice -- thank you @aprimadi
| ensure_not_set(&builder.options, "OPTIONS")?; | ||
| builder.options = Some(self.parse_options()?); | ||
| } else { | ||
| break; |
There was a problem hiding this comment.
I agree this would make a better user experience and I think it would be a good follow on project. I will file the ticket
alamb
changed the title
|
Thanks @aprimadi |
Which issue does this PR close?
Closes #6248
Rationale for this change
N/A
What changes are included in this PR?
Are these changes tested?
Yes
Are there any user-facing changes?
No