iffyio commented on code in PR #1949:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1949#discussion_r2248618331
##########
src/parser/mod.rs:
##########
@@ -16464,7 +16505,28 @@ impl<'a> Parser<'a> {
/// Parse [Statement::Return]
fn parse_return(&mut self) -> Result<Statement, ParserError> {
- match self.maybe_parse(|p| p.parse_expr())? {
+ let rs = self.maybe_parse(|p| {
+ let expr = p.parse_expr()?;
+
+ match &expr {
+ Expr::Value(_)
+ | Expr::Function(_)
+ | Expr::UnaryOp { .. }
+ | Expr::BinaryOp { .. }
+ | Expr::Case { .. }
+ | Expr::Cast { .. }
+ | Expr::Convert { .. }
+ | Expr::Subquery(_) => Ok(expr),
+ // todo: how to retstrict to variables?
Review Comment:
Ah right, makes sense that they're not newline dependent. To clarify what I
meant in the `IF/ELSE` scenario the idea would be to look whether the next
token can not be an expression - from a look at their docs the options seem
limited to
https://learn.microsoft.com/en-us/sql/t-sql/language-elements/control-of-flow?view=sql-server-ver15
```rust
if self.peek_one_of_keywords(
// hmm noticed while enumerating this list that the only ambiguous case
really
// seems to be IF - since offhand the others can't be an expression.
// So that the list might not be necessary after all.
[BEGIN, BREAK, CONTINUE, GOTO, IF, ELSE, RETURN, THROW, TRY, WAITFOR,
WHILE]
) {
// bare return statement
}
// else maybe_parse_expr
```
would something like this work?
--
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]
