mvzink commented on code in PR #1757:
URL: 
https://github.com/apache/datafusion-sqlparser-rs/pull/1757#discussion_r1985643698


##########
src/parser/mod.rs:
##########
@@ -10961,127 +10961,182 @@ impl<'a> Parser<'a> {
         })
     }
 
-    pub fn parse_set(&mut self) -> Result<Statement, ParserError> {
-        let modifier =
-            self.parse_one_of_keywords(&[Keyword::SESSION, Keyword::LOCAL, 
Keyword::HIVEVAR]);
-        if let Some(Keyword::HIVEVAR) = modifier {
-            self.expect_token(&Token::Colon)?;
-        } else if let Some(set_role_stmt) =
-            self.maybe_parse(|parser| parser.parse_set_role(modifier))?
-        {
-            return Ok(set_role_stmt);
+    fn parse_set_values(
+        &mut self,
+        parenthesized_assignment: bool,
+    ) -> Result<Vec<Expr>, ParserError> {
+        let mut values = vec![];
+
+        if parenthesized_assignment {
+            self.expect_token(&Token::LParen)?;
         }
 
-        let variables = if self.parse_keywords(&[Keyword::TIME, 
Keyword::ZONE]) {
-            OneOrManyWithParens::One(ObjectName::from(vec!["TIMEZONE".into()]))
-        } else if self.dialect.supports_parenthesized_set_variables()
+        loop {
+            let value = if let Some(expr) = self.try_parse_expr_sub_query()? {
+                expr
+            } else if let Ok(expr) = self.parse_expr() {
+                expr
+            } else {
+                self.expected("variable value", self.peek_token())?
+            };
+
+            values.push(value);
+            if self.consume_token(&Token::Comma) {
+                continue;
+            }
+
+            if parenthesized_assignment {
+                self.expect_token(&Token::RParen)?;
+            }
+            return Ok(values);
+        }
+    }
+
+    fn parse_set_assignment(
+        &mut self,
+    ) -> Result<(OneOrManyWithParens<ObjectName>, Expr), ParserError> {
+        let variables = if self.dialect.supports_parenthesized_set_variables()
             && self.consume_token(&Token::LParen)
         {
-            let variables = OneOrManyWithParens::Many(
+            let vars = OneOrManyWithParens::Many(
                 self.parse_comma_separated(|parser: &mut Parser<'a>| 
parser.parse_identifier())?
                     .into_iter()
                     .map(|ident| ObjectName::from(vec![ident]))
                     .collect(),
             );
             self.expect_token(&Token::RParen)?;
-            variables
+            vars
         } else {
             OneOrManyWithParens::One(self.parse_object_name(false)?)
         };
 
-        let names = matches!(&variables, OneOrManyWithParens::One(variable) if 
variable.to_string().eq_ignore_ascii_case("NAMES"));
+        if !(self.consume_token(&Token::Eq) || 
self.parse_keyword(Keyword::TO)) {
+            return self.expected("assignment operator", self.peek_token());
+        }
 
-        if names && self.dialect.supports_set_names() {
-            if self.parse_keyword(Keyword::DEFAULT) {
-                return Ok(Statement::SetNamesDefault {});
-            }
+        let values = self.parse_expr()?;
 
-            let charset_name = self.parse_identifier()?;
-            let collation_name = if 
self.parse_one_of_keywords(&[Keyword::COLLATE]).is_some() {
-                Some(self.parse_literal_string()?)
-            } else {
-                None
-            };
+        Ok((variables, values))
+    }
 
-            return Ok(Statement::SetNames {
-                charset_name,
-                collation_name,
-            });
+    fn parse_set(&mut self) -> Result<Statement, ParserError> {
+        let modifier =
+            self.parse_one_of_keywords(&[Keyword::SESSION, Keyword::LOCAL, 
Keyword::HIVEVAR]);
+
+        if let Some(Keyword::HIVEVAR) = modifier {
+            self.expect_token(&Token::Colon)?;
         }
 
-        let parenthesized_assignment = matches!(&variables, 
OneOrManyWithParens::Many(_));
+        if let Some(set_role_stmt) = self.maybe_parse(|parser| 
parser.parse_set_role(modifier))? {
+            return Ok(set_role_stmt);
+        }
 
-        if self.consume_token(&Token::Eq) || self.parse_keyword(Keyword::TO) {
-            if parenthesized_assignment {
-                self.expect_token(&Token::LParen)?;
-            }
+        if self.dialect.supports_comma_separated_set_assignments() {
+            if let Ok(v) =
+                self.try_parse(|parser| 
parser.parse_comma_separated(Parser::parse_set_assignment))
+            {

Review Comment:
   I think if you want to unify all the `Statement::Set*` variants into 
`Statement::Set(SetStatement)` (or similar), that might be better in a separate 
PR. However, adding a new variant for the new syntax I don't think will add 
that much complexity to the PR—not enough to warrant merging a partial 
implementation of the new syntax IMO. Nothing else here changes/touches the AST 
type definitions, and just making this one branch return the new variant won't 
make this any harder to review; in fact, it will be a lot clearer IMO.



-- 
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