iffyio commented on code in PR #1500:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1500#discussion_r1833051964
##########
src/parser/mod.rs:
##########
@@ -1271,10 +1271,22 @@ impl<'a> Parser<'a> {
)
}
};
- Ok(Expr::CompositeAccess {
- expr: Box::new(expr),
- key,
- })
+ if self.consume_token(&Token::LParen) {
+ self.prev_token();
+ let func = match
self.parse_function(ObjectName(vec![key]))? {
+ Expr::Function(func) => func,
+ _ => unreachable!(),
Review Comment:
Oh I think this panic won't be desirable here in any case given we don't
have an invariant and `self.parse_function` could be in its own right to return
something other than `Expr::Function` in a different situation.
Also given the nature of the syntax that we don't have any special keyword
to match on, feels a bit uneasy that we could end up parsing sql incorrectly in
other dialects in ambiguous cases.
could we do similar to e.g.
[try_parse_lambda](https://github.com/apache/datafusion-sqlparser-rs/blob/main/src/parser/mod.rs#L1332-L1346)
where this is both specific to mssql dialect, as well as potentially relying
on `self.maybe_parse` to bail out if the lookahead isn't what we expect?
##########
src/parser/mod.rs:
##########
@@ -1288,6 +1300,35 @@ impl<'a> Parser<'a> {
_ => self.expected("an expression", next_token),
}?;
+ // Composite function chain
+ while matches!(
+ expr,
+ Expr::Function(_)
+ | Expr::CompositeFunction { .. }
+ | Expr::Cast { .. }
+ | Expr::Convert { .. }
Review Comment:
hmm some of the entries seem a bit arbitary (like cast, convert)? I wonder
if instead of having this block as a special case, piggy-backing on my other
comment to do similar to the lambda parsing, we could probably have the chain
continuation logic in the same helper function that gets called once here (as
in the function knows to parse not just a single function but also
continuation/suffix if present of the full expression)
--
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]