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


##########
src/ast/mod.rs:
##########
@@ -195,14 +199,36 @@ impl fmt::Display for Ident {
 #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
-pub struct ObjectName(pub Vec<Ident>);
+pub struct ObjectName(pub Vec<ObjectNamePart>);
+
+impl From<Vec<Ident>> for ObjectName {
+    fn from(idents: Vec<Ident>) -> Self {
+        
ObjectName(idents.into_iter().map(ObjectNamePart::Identifier).collect())
+    }
+}
 
 impl fmt::Display for ObjectName {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "{}", display_separated(&self.0, "."))
     }
 }
 
+/// A single part of an ObjectName
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
+pub enum ObjectNamePart {
+    Identifier(Ident),
+}
+
+impl fmt::Display for ObjectNamePart {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            ObjectNamePart::Identifier(ident) => write!(f, "{}", ident),

Review Comment:
   I think the current is fine given that's the pattern used in the rest of the 
file



##########
src/parser/mod.rs:
##########
@@ -3605,6 +3605,24 @@ impl<'a> Parser<'a> {
         Ok(values)
     }
 
+    pub fn parse_period_separated<T, F>(&mut self, mut f: F) -> Result<Vec<T>, 
ParserError>
+    where
+        F: FnMut(&mut Parser<'a>) -> Result<T, ParserError>,
+    {
+        let mut values = vec![];
+        loop {
+            values.push(f(self)?);
+            if !self.consume_token(&Token::Period) {
+                break;
+            }
+        }
+        Ok(values)
+    }
+
+    pub fn parse_period_separated_identifiers(&mut self) -> Result<Vec<Ident>, 
ParserError> {

Review Comment:
   ```suggestion
       fn parse_period_separated_identifiers(&mut self) -> Result<Vec<Ident>, 
ParserError> {
   ```



##########
src/parser/mod.rs:
##########
@@ -10816,7 +10836,7 @@ impl<'a> Parser<'a> {
         self.expect_token(&Token::LParen)?;
         let aggregate_functions = 
self.parse_comma_separated(Self::parse_aliased_function_call)?;
         self.expect_keyword(Keyword::FOR)?;
-        let value_column = self.parse_object_name(false)?.0;
+        let value_column = self.parse_period_separated_identifiers()?;

Review Comment:
   ```suggestion
           let value_column = `self.parse_period_separated(|p| 
p.parse_identifier(false))?;
   ```
   Can we inline the code? thinking since the current method is only a wrapper 
around it



##########
src/parser/mod.rs:
##########
@@ -3605,6 +3605,24 @@ impl<'a> Parser<'a> {
         Ok(values)
     }
 
+    pub fn parse_period_separated<T, F>(&mut self, mut f: F) -> Result<Vec<T>, 
ParserError>

Review Comment:
   ```suggestion
       fn parse_period_separated<T, F>(&mut self, mut f: F) -> Result<Vec<T>, 
ParserError>
   ```
   Can we add a short description mentioning what the function does?



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