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


##########
src/ast/mod.rs:
##########
@@ -8433,6 +8430,31 @@ impl fmt::Display for SessionParamValue {
     }
 }
 
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
+pub enum SetVariableScope {
+    /// LOCAL (transaction or function) scope
+    Local,
+    /// SESSION scope
+    Session,
+    /// MySQL-specific `GLOBAL` scope
+    Global,
+    /// Not specified; usually implies SESSION
+    None,
+}
+
+impl fmt::Display for SetVariableScope {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Self::Local => write!(f, " LOCAL"),
+            Self::Session => write!(f, " SESSION"),
+            Self::Global => write!(f, " GLOBAL"),
+            Self::None => write!(f, ""),

Review Comment:
   does it work the same to define scope as an optional field (`scope: 
Option<SetVariableScope>`)? in order to represent `None` by convention?



##########
src/parser/mod.rs:
##########
@@ -10533,8 +10533,17 @@ 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]);
+        let modifier_keywords = if 
self.dialect.supports_global_variable_modifier() {

Review Comment:
   I'm thinking having the lone `global` keyword as a dialect flag might not 
scale API wise, to make it dialect specific maybe it might make sense to have 
delegate to the dialect? like with 
[parse_column_option](https://github.com/apache/datafusion-sqlparser-rs/blob/c0b5b20d11cee97c9541e6671a9833e1c2f926e7/src/dialect/mod.rs#L622)
 as an example except with a default implementation that others like hive and 
mysql can override. 
   e.g. 
   ```rust
   let scope = self.dialect.parse_set_variable_scope(self)?;
   ```



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