iffyio commented on code in PR #1500:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1500#discussion_r1835312544
##########
src/parser/mod.rs:
##########
@@ -1345,6 +1355,38 @@ impl<'a> Parser<'a> {
})
}
+ /// Parses method call expression
+ ///
+ /// Returns `Result<{parsed method-call-expr},{orig_expr}>`
+ fn try_parse_method(&mut self, orig_expr: Expr) -> Result<Result<Expr,
Expr>, ParserError> {
+ if !self.dialect.supports_methods() {
+ return Ok(Err(orig_expr));
+ }
+ let method_chain = self.maybe_parse(|p| {
Review Comment:
I think we can simplify the return type a bit if we do something like
```rust
let method_chain = self.maybe_parse(|p| {
let mut method_chain = Vec::new();
loop {
// ...
method_chain.push(func);
}
if method_chain.is_empty() {
p.expected("function")
} else {
Ok(method_chain)
}
})?;
if let Some(method_chain) {
Ok(Expr::Method{Method { expr, method_chain }})
} else {
Ok(expr)
}
```
that would remove the nested result type `Result<Result<Expr, Expr>>` ->
`Result<Expr>` as usual
##########
src/ast/mod.rs:
##########
@@ -808,6 +808,21 @@ pub enum Expr {
},
/// Scalar function call e.g. `LEFT(foo, 5)`
Function(Function),
+ /// Arbitrary expr method call
+ ///
+ /// Syntax:
+ ///
+ /// `<arbitrary-expr>.<arbitrary-expr>.<arbitrary-expr>...`
+ ///
+ /// > `arbitrary-expr` can be any expression including a function call.
+ ///
+ /// Example:
+ ///
+ /// ```sql
+ /// SELECT (SELECT ',' + name FROM sys.objects FOR XML PATH(''),
TYPE).value('.','NVARCHAR(MAX)')
+ /// SELECT
CONVERT(XML,'<Book>abc</Book>').value('.','NVARCHAR(MAX)').value('.','NVARCHAR(MAX)')
Review Comment:
Ah my bad, wrong link I was supposed to paste this one
https://learn.microsoft.com/en-us/sql/t-sql/xml/xml-data-type-methods?view=sql-server-ver16
in that it mentions explicitly the methods functionality, even though the
doc is specific to xml
##########
src/parser/mod.rs:
##########
@@ -1258,23 +1258,28 @@ impl<'a> Parser<'a> {
}
};
self.expect_token(&Token::RParen)?;
- if !self.consume_token(&Token::Period) {
- Ok(expr)
- } else {
- let tok = self.next_token();
- let key = match tok.token {
- Token::Word(word) => word.to_ident(),
- _ => {
- return parser_err!(
- format!("Expected identifier, found: {tok}"),
- tok.location
- )
+ match self.try_parse_method(expr)? {
Review Comment:
left a suggestion to avoid the nested result, thinking with that it should
be possible to return the expr as is and then here we avoid the need to match:
e.g.
```rust
let expr = self.try_parse_method(expr)?;
if !self.consume_token(...) {
}
```
--
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]
