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` 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, 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: 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