MartinSahlen commented on code in PR #1657:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1657#discussion_r1917904556
##########
src/parser/mod.rs:
##########
@@ -7341,6 +7341,10 @@ impl<'a> Parser<'a> {
pub fn parse_options(&mut self, keyword: Keyword) ->
Result<Vec<SqlOption>, ParserError> {
if self.parse_keyword(keyword) {
self.expect_token(&Token::LParen)?;
+ if self.peek_token() == Token::RParen {
+ self.next_token();
+ return Ok(vec![]);
+ }
let options =
self.parse_comma_separated(Parser::parse_sql_option)?;
self.expect_token(&Token::RParen)?;
Review Comment:
Ok, I think I found the bug, if I do this:
```rust
pub fn parse_options(&mut self, keyword: Keyword) ->
Result<Vec<SqlOption>, ParserError> {
if self.parse_keyword(keyword) {
self.expect_token(&Token::LParen)?;
let options =
self.parse_comma_separated0(Parser::parse_sql_option, Token::RParen)?;
self.expect_token(&Token::RParen)?;
Ok(options)
} else {
Ok(vec![])
}
}
```
its is working. And this leads me to look at the `parse_comma_separated0`
implementation. Should this not also __consume__ the `end_token` parameter?
```rust
pub fn parse_comma_separated0<T, F>(
&mut self,
f: F,
end_token: Token,
) -> Result<Vec<T>, ParserError>
```
Ok I tried making this function consume the end_token but it seems to cause
more issues. I also saw that there is a `parse_options_with_keywords` that at
the moment will not accept an empty options, but I will not change or touch
that in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
