Code review feedback

Author: yoavcloud

src/ast/mod.rs

@@ -10131,6 +10131,14 @@ impl fmt::Display for MemberOf {
     }
 }
 
+/// Creates a user
+///
+/// Syntax:
+/// ```sql
+/// CREATE [OR REPLACE] USER [IF NOT EXISTS] <name> [OPTIONS]
+/// ```
+///
+/// [Snowflake](https://docs.snowflake.com/en/sql-reference/sql/create-user)
 #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]

src/parser/mod.rs

@@ -4721,7 +4721,7 @@ impl<'a> Parser<'a> {
         }
     }
 
-    pub fn parse_create_user(&mut self, or_replace: bool) -> Result<Statement, ParserError> {
+    fn parse_create_user(&mut self, or_replace: bool) -> Result<Statement, ParserError> {
         let if_not_exists = self.parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]);
         let name = self.parse_identifier()?;
         let options = self.parse_key_value_options(false, &[Keyword::WITH, Keyword::TAG])?;
@@ -16683,43 +16683,38 @@ impl<'a> Parser<'a> {
         Ok(options)
     }
 
-    // Parses a `KEY = VALUE` construct based on the specified key
+    /// Parses a `KEY = VALUE` construct based on the specified key
     pub(crate) fn parse_key_value_option(
         &mut self,
         key: Word,
     ) -> Result<KeyValueOption, ParserError> {
         self.expect_token(&Token::Eq)?;
-        if self.parse_keyword(Keyword::TRUE) {
-            Ok(KeyValueOption {
-                option_name: key.value,
-                option_type: KeyValueOptionType::BOOLEAN,
-                value: "TRUE".to_string(),
-            })
-        } else if self.parse_keyword(Keyword::FALSE) {
-            Ok(KeyValueOption {
+        match self.next_token().token {
+            Token::SingleQuotedString(value) => Ok(KeyValueOption {
                 option_name: key.value,
-                option_type: KeyValueOptionType::BOOLEAN,
-                value: "FALSE".to_string(),
-            })
-        } else {
-            match self.next_token().token {
-                Token::SingleQuotedString(value) => Ok(KeyValueOption {
-                    option_name: key.value,
-                    option_type: KeyValueOptionType::STRING,
-                    value,
-                }),
-                Token::Word(word) => Ok(KeyValueOption {
-                    option_name: key.value,
-                    option_type: KeyValueOptionType::ENUM,
-                    value: word.value,
-                }),
-                Token::Number(n, _) => Ok(KeyValueOption {
+                option_type: KeyValueOptionType::STRING,
+                value,
+            }),
+            Token::Word(word)
+                if word.keyword == Keyword::TRUE || word.keyword == Keyword::FALSE =>
+            {
+                Ok(KeyValueOption {
                     option_name: key.value,
-                    option_type: KeyValueOptionType::NUMBER,
-                    value: n,
-                }),
-                _ => self.expected("expected option value", self.peek_token()),
+                    option_type: KeyValueOptionType::BOOLEAN,
+                    value: word.value.to_uppercase(),
+                })
             }
+            Token::Word(word) => Ok(KeyValueOption {
+                option_name: key.value,
+                option_type: KeyValueOptionType::ENUM,
+                value: word.value,
+            }),
+            Token::Number(n, _) => Ok(KeyValueOption {
+                option_name: key.value,
+                option_type: KeyValueOptionType::NUMBER,
+                value: n,
+            }),
+            _ => self.expected("expected option value", self.peek_token()),
         }
     }
 }

tests/sqlparser_common.rs

@@ -27,6 +27,8 @@ extern crate core;
 
 use helpers::attached_token::AttachedToken;
 use matches::assert_matches;
+use sqlparser::ast::helpers::key_value_options::*;
+use sqlparser::ast::helpers::key_value_options::{KeyValueOptions, KeyValueOptionsDelimiter};
 use sqlparser::ast::SelectItem::UnnamedExpr;
 use sqlparser::ast::TableFactor::{Pivot, Unpivot};
 use sqlparser::ast::*;
@@ -16259,13 +16261,69 @@ fn parse_notnull() {
 
 #[test]
 fn parse_create_user() {
-    verified_stmt("CREATE USER u1");
+    let create = verified_stmt("CREATE USER u1");
+    match create {
+        Statement::CreateUser(stmt) => {
+            assert_eq!(stmt.name, Ident::new("u1"));
+        }
+        _ => unreachable!(),
+    }
     verified_stmt("CREATE OR REPLACE USER u1");
     verified_stmt("CREATE OR REPLACE USER IF NOT EXISTS u1");
     verified_stmt("CREATE OR REPLACE USER IF NOT EXISTS u1 PASSWORD='secret'");
     verified_stmt(
         "CREATE OR REPLACE USER IF NOT EXISTS u1 PASSWORD='secret' MUST_CHANGE_PASSWORD=TRUE",
     );
     verified_stmt("CREATE OR REPLACE USER IF NOT EXISTS u1 PASSWORD='secret' MUST_CHANGE_PASSWORD=TRUE TYPE=SERVICE TAG (t1='v1')");
-    verified_stmt("CREATE OR REPLACE USER IF NOT EXISTS u1 PASSWORD='secret' MUST_CHANGE_PASSWORD=TRUE TYPE=SERVICE WITH TAG (t1='v1', t2='v2')");
+    let create = verified_stmt("CREATE OR REPLACE USER IF NOT EXISTS u1 PASSWORD='secret' MUST_CHANGE_PASSWORD=TRUE TYPE=SERVICE WITH TAG (t1='v1', t2='v2')");
+    match create {
+        Statement::CreateUser(stmt) => {
+            assert_eq!(stmt.name, Ident::new("u1"));
+            assert_eq!(stmt.or_replace, true);
+            assert_eq!(stmt.if_not_exists, true);
+            assert_eq!(
+                stmt.options,
+                KeyValueOptions {
+                    delimiter: KeyValueOptionsDelimiter::Space,
+                    options: vec![
+                        KeyValueOption {
+                            option_name: "PASSWORD".to_string(),
+                            value: "secret".to_string(),
+                            option_type: KeyValueOptionType::STRING
+                        },
+                        KeyValueOption {
+                            option_name: "MUST_CHANGE_PASSWORD".to_string(),
+                            value: "TRUE".to_string(),
+                            option_type: KeyValueOptionType::BOOLEAN
+                        },
+                        KeyValueOption {
+                            option_name: "TYPE".to_string(),
+                            value: "SERVICE".to_string(),
+                            option_type: KeyValueOptionType::ENUM
+                        },
+                    ],
+                },
+            );
+            assert_eq!(stmt.with_tags, true);
+            assert_eq!(
+                stmt.tags,
+                KeyValueOptions {
+                    delimiter: KeyValueOptionsDelimiter::Comma,
+                    options: vec![
+                        KeyValueOption {
+                            option_name: "t1".to_string(),
+                            value: "v1".to_string(),
+                            option_type: KeyValueOptionType::STRING
+                        },
+                        KeyValueOption {
+                            option_name: "t2".to_string(),
+                            value: "v2".to_string(),
+                            option_type: KeyValueOptionType::STRING
+                        },
+                    ]
+                }
+            );
+        }
+        _ => unreachable!(),
+    }
 }