aharpervc commented on code in PR #1949:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1949#discussion_r2220430113
##########
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:
> I think the error is inappropriate in this case
What do you suggest?
> This seems more like a semantic check -- this crate is focused on syntax
parsing.
The difficulty here is that without semicolon tokens the return statement is
ambiguous. Consider the following perfectly valid T-SQL statement (which is
also a test case example) which has several variations of `return`:
```
CREATE OR ALTER PROCEDURE example_sp
AS
IF USER_NAME() = 'X'
RETURN
IF 1 = 2
RETURN (SELECT 1)
RETURN CONVERT(INT, 123)
```
If you revert this change and run that test, you get this error:
```
thread 'test_supports_statements_without_semicolon_delimiter' panicked at
tests/sqlparser_mssql.rs:2533:14:
called `Result::unwrap()` on an `Err` value: ParserError("Expected: an SQL
statement, found: 1 at Line: 1, Column: 72")
```
The reason is that for the first `return`, the `self.maybe_parse(|p|
p.parse_expr())?` "successfully" parses/consumes the `IF` keyword. However,
this is contrary to the keyword documentation for SQL Server
([ref](https://learn.microsoft.com/en-us/sql/t-sql/language-elements/return-transact-sql?view=sql-server-ver15))
which requires an "integer expression".
I think I had said when introducing parse_return in a previous PR that we'd
have to come back and tighten it up eventually, but I can't find that
discussion 😐.
--
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
