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


##########
src/ast/mod.rs:
##########
@@ -7377,15 +7420,84 @@ pub enum MySQLColumnPosition {
 impl Display for MySQLColumnPosition {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
-            MySQLColumnPosition::First => Ok(write!(f, "FIRST")?),
+            MySQLColumnPosition::First => write!(f, "FIRST"),
             MySQLColumnPosition::After(ident) => {
                 let column_name = &ident.value;
-                Ok(write!(f, "AFTER {column_name}")?)
+                write!(f, "AFTER {column_name}")
             }
         }
     }
 }
 
+/// MySQL `CREATE VIEW` algorithm parameter: [ALGORITHM = {UNDEFINED | MERGE | 
TEMPTABLE}]
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
+pub enum CreateViewAlgorithm {
+    Undefined,
+    Merge,
+    TempTable,
+}
+
+impl Display for CreateViewAlgorithm {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            CreateViewAlgorithm::Undefined => write!(f, "UNDEFINED"),
+            CreateViewAlgorithm::Merge => write!(f, "MERGE"),
+            CreateViewAlgorithm::TempTable => write!(f, "TEMPTABLE"),
+        }
+    }
+}
+/// MySQL `CREATE VIEW` security parameter: [SQL SECURITY { DEFINER | INVOKER 
}]
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
+pub enum CreateViewSecurity {
+    Definer,
+    Invoker,
+}
+
+impl Display for CreateViewSecurity {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            CreateViewSecurity::Definer => write!(f, "DEFINER"),
+            CreateViewSecurity::Invoker => write!(f, "INVOKER"),
+        }
+    }
+}
+
+/// [MySQL] `CREATE VIEW` additional parameters
+///
+/// [MySQL]: https://dev.mysql.com/doc/refman/9.1/en/create-view.html
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
+pub struct CreateViewParams {
+    pub algorithm: Option<CreateViewAlgorithm>,
+    pub definer: Option<Grantee>,
+    pub security: Option<CreateViewSecurity>,
+}
+
+impl Display for CreateViewParams {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let parts = [
+            self.algorithm
+                .as_ref()
+                .map(|algorithm| format!("ALGORITHM = {algorithm}")),
+            self.definer
+                .as_ref()
+                .map(|definer| format!("DEFINER = {definer}")),
+            self.security
+                .as_ref()
+                .map(|security| format!("SQL SECURITY {security}")),
+        ]
+        .into_iter()
+        .flatten()
+        .collect::<Vec<_>>();
+        display_separated(&parts, " ").fmt(f)

Review Comment:
   Can we write these explicitly as e.g?
   ```rust
   if let Some(algorithm) = self.algorithm { write!() }
   ```
   I think that potentially makes it easier to spot the final output and extend 
in the future.
   Also ideally we can use an exhaustive check with
   ```rust
   let CreateviewParams { algorithm, definer, security } = self
   ```
   so that the user is automatically guided to this function whenever the 
representation changes



##########
src/tokenizer.rs:
##########
@@ -1202,6 +1202,9 @@ impl<'a> Tokenizer<'a> {
                             }
                         }
                         Some(' ') => Ok(Some(Token::AtSign)),
+                        Some('\'') => Ok(Some(Token::AtSign)),
+                        Some('\"') => Ok(Some(Token::AtSign)),
+                        Some('`') => Ok(Some(Token::AtSign)),

Review Comment:
   Ah I see that makes sense to me and the current approach sounds reasonable! 
Can we add a comment around mentioning the example use case? I think that would 
be useful since the logic/tradeoff wouldnt be obvious otherwise, Also can we 
extend the tokenizer test case to cover the other two tokens  supported by this 
path `"` and '`'?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to