iffyio commented on code in PR #1656:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1656#discussion_r1917322346
##########
src/parser/mod.rs:
##########
@@ -13150,6 +13151,40 @@ impl<'a> Parser<'a> {
}
}
+ pub fn parse_raiserror(&mut self) -> Result<Statement, ParserError> {
+ self.expect_token(&Token::LParen)?;
+ let message = Box::new(self.parse_expr()?);
+ self.expect_token(&Token::Comma)?;
+ let severity = Box::new(self.parse_expr()?);
+ self.expect_token(&Token::Comma)?;
+ let state = Box::new(self.parse_expr()?);
+ let mut arguments = vec![];
+ if self.consume_token(&Token::Comma) {
+ arguments = self.parse_comma_separated(Parser::parse_expr)?;
+ }
+ self.expect_token(&Token::RParen)?;
+ let mut options = vec![];
+ if self.parse_keyword(Keyword::WITH) {
+ options =
self.parse_comma_separated(Parser::parse_raiserror_option)?;
+ }
+ Ok(Statement::RaisError {
+ message,
+ severity,
+ state,
+ arguments,
+ options,
+ })
+ }
+
+ pub fn parse_raiserror_option(&mut self) -> Result<RaisErrorOption,
ParserError> {
+ match self.expect_one_of_keywords(&[Keyword::LOG, Keyword::NOWAIT,
Keyword::SETERROR])? {
+ Keyword::LOG => Ok(RaisErrorOption::Log),
+ Keyword::NOWAIT => Ok(RaisErrorOption::NoWait),
+ Keyword::SETERROR => Ok(RaisErrorOption::SetError),
+ _ => unreachable!(),
Review Comment:
We can return an error with `self.expected(...)` here instead of panicking,
that way if there's a bug in the code we avoid a crash
##########
src/parser/mod.rs:
##########
@@ -13150,6 +13151,40 @@ impl<'a> Parser<'a> {
}
}
+ pub fn parse_raiserror(&mut self) -> Result<Statement, ParserError> {
+ self.expect_token(&Token::LParen)?;
+ let message = Box::new(self.parse_expr()?);
+ self.expect_token(&Token::Comma)?;
+ let severity = Box::new(self.parse_expr()?);
+ self.expect_token(&Token::Comma)?;
+ let state = Box::new(self.parse_expr()?);
+ let mut arguments = vec![];
+ if self.consume_token(&Token::Comma) {
+ arguments = self.parse_comma_separated(Parser::parse_expr)?;
+ }
+ self.expect_token(&Token::RParen)?;
+ let mut options = vec![];
+ if self.parse_keyword(Keyword::WITH) {
+ options =
self.parse_comma_separated(Parser::parse_raiserror_option)?;
+ }
+ Ok(Statement::RaisError {
+ message,
+ severity,
+ state,
+ arguments,
+ options,
+ })
+ }
+
+ pub fn parse_raiserror_option(&mut self) -> Result<RaisErrorOption,
ParserError> {
Review Comment:
```suggestion
fn parse_raiserror_option(&mut self) -> Result<RaisErrorOption,
ParserError> {
```
maybe this function is worth inlining given that its somewhat trivial and
only used once?
##########
src/parser/mod.rs:
##########
@@ -13150,6 +13151,40 @@ impl<'a> Parser<'a> {
}
}
+ pub fn parse_raiserror(&mut self) -> Result<Statement, ParserError> {
+ self.expect_token(&Token::LParen)?;
+ let message = Box::new(self.parse_expr()?);
+ self.expect_token(&Token::Comma)?;
+ let severity = Box::new(self.parse_expr()?);
+ self.expect_token(&Token::Comma)?;
+ let state = Box::new(self.parse_expr()?);
+ let mut arguments = vec![];
+ if self.consume_token(&Token::Comma) {
+ arguments = self.parse_comma_separated(Parser::parse_expr)?;
+ }
+ self.expect_token(&Token::RParen)?;
+ let mut options = vec![];
+ if self.parse_keyword(Keyword::WITH) {
+ options =
self.parse_comma_separated(Parser::parse_raiserror_option)?;
+ }
Review Comment:
```suggestion
let mut options = vec![];
let options = if self.parse_keyword(Keyword::WITH) {
self.parse_comma_separated(Parser::parse_raiserror_option)?;
} else {
vec![]
}
```
##########
tests/sqlparser_mssql.rs:
##########
@@ -1250,6 +1250,27 @@ fn parse_mssql_declare() {
);
}
+#[test]
+fn test_parse_raiserror() {
+ let sql = r#"RAISERROR('This is a test', 16, 1)"#;
Review Comment:
Since this introduces a new node into the AST, could we add one scenario
that asserts the body of the returned AST? in a style similar to
[this](https://github.com/apache/datafusion-sqlparser-rs/blob/810c70db77322cd5eaf7424e947973d8c111a816/tests/sqlparser_mssql.rs#L589-L602)
for example
##########
src/ast/mod.rs:
##########
@@ -3441,6 +3441,37 @@ pub enum Statement {
///
/// See
<https://learn.microsoft.com/en-us/sql/t-sql/statements/set-statements-transact-sql>
SetSessionParam(SetSessionParamKind),
+ /// RaiseError (MSSQL)
Review Comment:
Can we add a link in the doc to the syntax? that would make it easier to
find when folks come across the variant.
Also the identation of the variant seems off, the PR might need a `cargo
fmt` run?
##########
src/parser/mod.rs:
##########
@@ -13150,6 +13151,40 @@ impl<'a> Parser<'a> {
}
}
+ pub fn parse_raiserror(&mut self) -> Result<Statement, ParserError> {
+ self.expect_token(&Token::LParen)?;
+ let message = Box::new(self.parse_expr()?);
+ self.expect_token(&Token::Comma)?;
+ let severity = Box::new(self.parse_expr()?);
+ self.expect_token(&Token::Comma)?;
+ let state = Box::new(self.parse_expr()?);
+ let mut arguments = vec![];
+ if self.consume_token(&Token::Comma) {
+ arguments = self.parse_comma_separated(Parser::parse_expr)?;
+ }
Review Comment:
```suggestion
let mut arguments = vec![];
let arguments = if self.consume_token(&Token::Comma) {
arguments = self.parse_comma_separated(Parser::parse_expr)?;
} else {
vec![]
}
```
in order to avoid the mutable variable
##########
src/parser/mod.rs:
##########
@@ -13150,6 +13151,40 @@ impl<'a> Parser<'a> {
}
}
+ pub fn parse_raiserror(&mut self) -> Result<Statement, ParserError> {
Review Comment:
Could we add a description to the function given that it's a public
function? We can have it mention the `[Statement::RaisError]` variant that it
returns
--
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]
