iffyio commented on code in PR #1809:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1809#discussion_r2056434400
##########
src/parser/mod.rs:
##########
@@ -484,8 +488,18 @@ impl<'a> Parser<'a> {
}
let statement = self.parse_statement()?;
+ expecting_statement_delimiter = match &statement {
+ Statement::If(s) => match &s.if_block.conditional_statements {
+ // the `END` keyword doesn't need to be followed by a
statement delimiter, so it shouldn't be expected here
+ ConditionalStatements::BeginEnd { .. } => false,
+ // parsing the statement sequence consumes the statement
delimiter, so it shouldn't be expected here
+ ConditionalStatements::Sequence { .. } => false,
+ },
+ // Treat batch delimiter as an end of statement, so no
additional statement delimiter expected here
Review Comment:
will this be replaced by similar solution as in #1808?
##########
src/dialect/mssql.rs:
##########
@@ -116,7 +116,17 @@ impl Dialect for MsSqlDialect {
true
}
- fn is_column_alias(&self, kw: &Keyword, _parser: &mut Parser) -> bool {
+ fn is_column_alias(&self, kw: &Keyword, parser: &mut Parser) -> bool {
+ // if we find maybe whitespace then a newline looking backward, then
`GO` ISN'T a column alias
+ // if we can't find a newline then we assume that `GO` IS a column
alias
+ if kw == &Keyword::GO
+ && parser
+ .expect_previously_only_whitespace_until_newline()
+ .is_ok()
Review Comment:
Ah I see, 2. sounds desirable to return a boolean, in that the function is
only checking if a condition holds, then interpretation of the condition is
left to the caller of the function. in the other usage the caller does e.g.
```rust
if !self.prev_whitespace_only_is_newline() {
return self.expected("newline ...")
}
```
##########
src/parser/mod.rs:
##########
@@ -3939,6 +3954,26 @@ impl<'a> Parser<'a> {
})
}
+ /// Return nth previous token, possibly whitespace
+ /// (or [`Token::EOF`] when before the beginning of the stream).
+ pub fn peek_prev_nth_token_no_skip(&self, n: usize) -> TokenWithSpan {
Review Comment:
```suggestion
pub(crate) fn peek_prev_nth_token_no_skip(&self, n: usize) ->
&TokenWithSpan {
```
we can probably start with the API being non-public. Then can we switch to
returning a reference to avoid the mandatory cloning?
##########
src/parser/mod.rs:
##########
@@ -618,6 +632,7 @@ impl<'a> Parser<'a> {
// `COMMENT` is snowflake specific
https://docs.snowflake.com/en/sql-reference/sql/comment
Keyword::COMMENT if self.dialect.supports_comment_on() =>
self.parse_comment(),
Keyword::PRINT => self.parse_print(),
+ Keyword::GO => self.parse_go(),
Review Comment:
Indeed the code is inconsistent in that regard historically, for newer
statements ideally the parser functions are self contained when possible
##########
src/parser/mod.rs:
##########
@@ -4055,6 +4090,38 @@ impl<'a> Parser<'a> {
)
}
+ /// Look backwards in the token stream and expect that there was only
whitespace tokens until the previous newline or beginning of string
+ pub(crate) fn expect_previously_only_whitespace_until_newline(
+ &mut self,
+ ) -> Result<(), ParserError> {
+ let mut look_back_count = 1;
+ loop {
+ let prev_token = self.peek_prev_nth_token_no_skip(look_back_count);
+ match prev_token.token {
+ Token::EOF => break,
+ Token::Whitespace(ref w) => match w {
+ Whitespace::Newline => break,
+ // special consideration required for single line comments
since that string includes the newline
+ Whitespace::SingleLineComment { comment, prefix: _ } => {
+ if comment.ends_with('\n') {
Review Comment:
double checking: is there a scenario where a single line comment doesn't end
with a new line? spontaneously sounds like that should always hold true so that
the manual newline check would not be required
##########
src/parser/mod.rs:
##########
@@ -15064,6 +15117,38 @@ impl<'a> Parser<'a> {
}))
}
+ /// Parse [Statement::Go]
+ fn parse_go(&mut self) -> Result<Statement, ParserError> {
+ self.expect_previously_only_whitespace_until_newline()?;
Review Comment:
Ah that makes sense! And the added comment clarifies as well thanks!
--
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
