iffyio commented on code in PR #1836: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1836#discussion_r2072414916
########## src/parser/mod.rs: ########## @@ -12987,23 +12991,34 @@ impl<'a> Parser<'a> { /// Parse a GRANT statement. pub fn parse_grant(&mut self) -> Result<Statement, ParserError> { - let (privileges, objects) = self.parse_grant_revoke_privileges_objects()?; + let (privileges, objects) = self.parse_grant_deny_revoke_privileges_objects()?; self.expect_keyword_is(Keyword::TO)?; let grantees = self.parse_grantees()?; let with_grant_option = self.parse_keywords(&[Keyword::WITH, Keyword::GRANT, Keyword::OPTION]); - let granted_by = self - .parse_keywords(&[Keyword::GRANTED, Keyword::BY]) - .then(|| self.parse_identifier().unwrap()); + let as_grantor = if self.peek_keyword(Keyword::AS) { + self.parse_keywords(&[Keyword::AS]) + .then(|| self.parse_identifier().unwrap()) Review Comment: Oh I realise this unwrap wasn't introduced by this PR, but can we switch it to return an error instead? it looks otherwise like an scenario where the parser will panic on invalid sql which would be undesirable ########## src/parser/mod.rs: ########## @@ -12987,23 +12991,34 @@ impl<'a> Parser<'a> { /// Parse a GRANT statement. pub fn parse_grant(&mut self) -> Result<Statement, ParserError> { - let (privileges, objects) = self.parse_grant_revoke_privileges_objects()?; + let (privileges, objects) = self.parse_grant_deny_revoke_privileges_objects()?; self.expect_keyword_is(Keyword::TO)?; let grantees = self.parse_grantees()?; let with_grant_option = self.parse_keywords(&[Keyword::WITH, Keyword::GRANT, Keyword::OPTION]); - let granted_by = self - .parse_keywords(&[Keyword::GRANTED, Keyword::BY]) - .then(|| self.parse_identifier().unwrap()); + let as_grantor = if self.peek_keyword(Keyword::AS) { + self.parse_keywords(&[Keyword::AS]) + .then(|| self.parse_identifier().unwrap()) + } else { + None + }; + + let granted_by = if self.peek_keywords(&[Keyword::GRANTED, Keyword::BY]) { + self.parse_keywords(&[Keyword::GRANTED, Keyword::BY]) + .then(|| self.parse_identifier().unwrap()) Review Comment: similar here, we would want to return an error vs unwrap ########## src/parser/mod.rs: ########## @@ -13409,9 +13430,40 @@ impl<'a> Parser<'a> { } } + /// Parse [`Statement::Deny`] + pub fn parse_deny(&mut self) -> Result<Statement, ParserError> { + self.expect_keyword(Keyword::DENY)?; + + let (privileges, objects) = self.parse_grant_deny_revoke_privileges_objects()?; + let objects = match objects { + Some(o) => o, + None => { + return parser_err!( + "DENY statements must specify an object", + self.peek_token().span.start + ) + } + }; + + self.expect_keyword_is(Keyword::TO)?; + let grantees = self.parse_grantees()?; + let cascade = self.parse_cascade_option(); + let granted_by = self + .parse_keywords(&[Keyword::AS]) + .then(|| self.parse_identifier().unwrap()); Review Comment: same comment here re unwrap ########## src/parser/mod.rs: ########## @@ -13020,14 +13035,18 @@ impl<'a> Parser<'a> { GranteesType::Share } else if self.parse_keyword(Keyword::GROUP) { GranteesType::Group - } else if self.parse_keyword(Keyword::PUBLIC) { - GranteesType::Public } else if self.parse_keywords(&[Keyword::DATABASE, Keyword::ROLE]) { GranteesType::DatabaseRole } else if self.parse_keywords(&[Keyword::APPLICATION, Keyword::ROLE]) { GranteesType::ApplicationRole } else if self.parse_keyword(Keyword::APPLICATION) { GranteesType::Application + } else if self.peek_keyword(Keyword::PUBLIC) { + if dialect_of!(self is MsSqlDialect) { + grantee_type + } else { + GranteesType::Public + } Review Comment: hmm not sure I followed the intention here, public is represented differently for mssql? (we would want to describe that behavior without the `dialect_of` macro I think) -- 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