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


##########
src/ast/mod.rs:
##########
@@ -8023,6 +8023,9 @@ pub enum FunctionArgOperator {
     Colon,
     /// function(arg1 VALUE value1)
     Value,
+    /// function(arg1 value1), with no operator between the name and the value,
+    /// as in PostgreSQL `XMLPARSE(DOCUMENT value)`

Review Comment:
   Can we add a link to the docs describing the syntax?



##########
src/parser/mod.rs:
##########
@@ -2531,8 +2531,47 @@ impl<'a> Parser<'a> {
         })
     }
 
+    /// Parse `XMLPARSE({ DOCUMENT | CONTENT } value)`, whose mode keyword is
+    /// carried by the single argument as a name with no operator. Neither mode
+    /// word is reserved, so both arrive as plain words rather than keywords.
+    fn parse_xmlparse_call(&mut self, name: ObjectName) -> Result<Function, 
ParserError> {
+        self.expect_token(&Token::LParen)?;
+        let is_mode = matches!(&self.peek_token_ref().token, Token::Word(word)
+            if word.quote_style.is_none()
+                && (word.value.eq_ignore_ascii_case("content")
+                    || word.value.eq_ignore_ascii_case("document")));
+        if !is_mode {
+            return self.expected_ref("CONTENT or DOCUMENT", 
self.peek_token_ref());
+        }
+        let arg = FunctionArg::Named {
+            name: self.parse_identifier()?,
+            arg: FunctionArgExpr::Expr(self.parse_expr()?),
+            operator: FunctionArgOperator::Space,
+        };
+        self.expect_token(&Token::RParen)?;
+        Ok(Function {
+            name,
+            uses_odbc_syntax: false,
+            parameters: FunctionArguments::None,
+            args: FunctionArguments::List(FunctionArgumentList {
+                duplicate_treatment: None,
+                args: vec![arg],
+                clauses: vec![],
+            }),
+            filter: None,
+            null_treatment: None,
+            over: None,
+            within_group: vec![],
+        })
+    }
+
     /// Parse a function call expression named by `name` and return it as an 
`Expr`.
     pub fn parse_function(&mut self, name: ObjectName) -> Result<Expr, 
ParserError> {
+        if self.dialect.supports_xml_expressions()
+            && Self::is_simple_unquoted_object_name(&name, "xmlparse")
+        {
+            return self.parse_xmlparse_call(name).map(Expr::Function);
+        }

Review Comment:
   I think we can simply by moving inside the parse_function_call - 
[here](https://github.com/LucaCappelletti94/sqlparser-rs/blob/78a68230eadb9298b5931461561d34f852bef4d2/src/parser/mod.rs#L2598)
 we could do 
   
   ```
   let mut args = if dialect.supports_xml_expressions() && name = "xmlparse" {
         ...
   } else {
       parse_function_argument_list
   }
   ```
   



##########
src/parser/mod.rs:
##########
@@ -2531,8 +2531,47 @@ impl<'a> Parser<'a> {
         })
     }
 
+    /// Parse `XMLPARSE({ DOCUMENT | CONTENT } value)`, whose mode keyword is
+    /// carried by the single argument as a name with no operator. Neither mode
+    /// word is reserved, so both arrive as plain words rather than keywords.
+    fn parse_xmlparse_call(&mut self, name: ObjectName) -> Result<Function, 
ParserError> {
+        self.expect_token(&Token::LParen)?;
+        let is_mode = matches!(&self.peek_token_ref().token, Token::Word(word)
+            if word.quote_style.is_none()
+                && (word.value.eq_ignore_ascii_case("content")
+                    || word.value.eq_ignore_ascii_case("document")));
+        if !is_mode {
+            return self.expected_ref("CONTENT or DOCUMENT", 
self.peek_token_ref());
+        }

Review Comment:
   impl wise these should be declared and parsed as keywords if they're missing 
in the list?



##########
src/parser/mod.rs:
##########
@@ -2531,8 +2531,47 @@ impl<'a> Parser<'a> {
         })
     }
 
+    /// Parse `XMLPARSE({ DOCUMENT | CONTENT } value)`, whose mode keyword is
+    /// carried by the single argument as a name with no operator. Neither mode
+    /// word is reserved, so both arrive as plain words rather than keywords.
+    fn parse_xmlparse_call(&mut self, name: ObjectName) -> Result<Function, 
ParserError> {
+        self.expect_token(&Token::LParen)?;
+        let is_mode = matches!(&self.peek_token_ref().token, Token::Word(word)
+            if word.quote_style.is_none()
+                && (word.value.eq_ignore_ascii_case("content")
+                    || word.value.eq_ignore_ascii_case("document")));
+        if !is_mode {
+            return self.expected_ref("CONTENT or DOCUMENT", 
self.peek_token_ref());
+        }

Review Comment:
   oh actually, since we're hardcoding property of the xmlparse and it requires 
an argument, I don't think either keyword or this code is needed, the parse can 
just consume the next two tokens as space separated words (the info or content 
vs document is already being dropped before reaching the AST so the validation 
isn't necessary at this layer)



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