iffyio commented on code in PR #1474:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1474#discussion_r1798024291
##########
src/parser/mod.rs:
##########
@@ -9416,27 +9416,42 @@ impl<'a> Parser<'a> {
}
}
+ fn parse_set_role(&mut self, modifier: Option<Keyword>) ->
Option<Statement> {
Review Comment:
thinking maybe this function should return a Result instead as usual, the
current rewind behavior doesn't seem correct otherwise?
If this part throws an error, it would have already consumed some tokens
from the stream.
```rust
role_name: Some(self.parse_identifier(false).ok()?),
```
So that the assumption being made at the caller here becomes incorrect
```rust
self.prev_token(); // replace the "role" token which was
actually a variable name
```
(Can we add a test case demonstrating this expected behavior in any case?)
Thinking if `parse_set_role` is a normal parse function, then the caller can
instead do `self.maybe_parse(parse_set_role)` which will do the automatic
rewind if it turns out not to be a legit `SET ROLE` statement in any scenario?
##########
src/parser/mod.rs:
##########
@@ -9416,27 +9416,42 @@ impl<'a> Parser<'a> {
}
}
+ fn parse_set_role(&mut self, modifier: Option<Keyword>) ->
Option<Statement> {
+ let context_modifier = match modifier {
+ Some(Keyword::LOCAL) => ContextModifier::Local,
+ Some(Keyword::SESSION) => ContextModifier::Session,
+ _ => ContextModifier::None,
+ };
+
+ if self.parse_keyword(Keyword::NONE) {
+ Some(Statement::SetRole {
+ context_modifier,
+ role_name: None,
+ })
+ } else if matches!(
+ self.peek_token().token,
+ Token::Word(_) | Token::DoubleQuotedString(_) |
Token::SingleQuotedString(_)
+ ) {
Review Comment:
Not sure I followed the intention with this change, it seems to match the
behavior of `parse_identifier()`?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
