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

Reply via email to