Re: [PR] PostgreSQL: ALTER USER password option [datafusion-sqlparser-rs]

["iffyio (via GitHub)" <gi...@apache.org>]

iffyio commented on code in PR #2142:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2142#discussion_r2675457290


##########
src/ast/mod.rs:
##########
@@ -10046,12 +10046,15 @@ impl fmt::Display for CreateUser {
 
 /// Modifies the properties of a user
 ///
-/// Syntax:
+/// [Snowflake Syntax]:
 /// ```sql
 /// ALTER USER [ IF EXISTS ] [ <name> ] [ OPTIONS ]
 /// ```
 ///
-/// [Snowflake](https://docs.snowflake.com/en/sql-reference/sql/alter-user)

Review Comment:
   It seems we dropped this doc reference, could we add it back together with the new pg reference?



##########
src/parser/alter.rs:
##########
@@ -292,6 +293,24 @@ impl Parser<'_> {
             vec![]
         };
 
+        let encrypted = self.parse_keyword(Keyword::ENCRYPTED);
+        let password = if self.parse_keyword(Keyword::PASSWORD) {
+            if self.parse_keyword(Keyword::NULL) {
+                Some(AlterUserPassword {
+                    encrypted,
+                    password: None,
+                })
+            } else {
+                let password = self.parse_literal_string()?;
+                Some(AlterUserPassword {
+                    encrypted,
+                    password: Some(password),
+                })
+            }

Review Comment:
   minor probably to clarify the code, we could do e.g.
   
   ```rust
   let password = if self.parse_keyword(Keyword::NULL) {
       None
   } else {
       Some(self.parse_literal_string()?)
   };
   Some(AlterUserPassword {
       encrypted,
       password,
   })
   ```



##########
tests/sqlparser_common.rs:
##########
@@ -17885,6 +17885,12 @@ fn test_parse_alter_user() {
         _ => unreachable!(),
     }
     verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('ALL'), PASSWORD='secret', WORKLOAD_IDENTITY=(TYPE=AWS, ARN='arn:aws:iam::123456789:r1/')");
+
+    verified_stmt("ALTER USER u1 PASSWORD 'AAA'");

Review Comment:
   can we add test cases for `ENCRYPTED` and `PASSWORD NULL`?



-- 
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-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org