iffyio commented on code in PR #1831:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1831#discussion_r2067185362
##########
src/test_utils.rs:
##########
@@ -166,6 +168,30 @@ impl TestedDialects {
only_statement
}
+ /// The same as [`one_statement_parses_to`] but it works for a multiple
statements
+ pub fn statements_parse_to(
+ &self,
+ sql: &str,
+ statement_count: usize,
+ canonical: &str,
+ ) -> Vec<Statement> {
+ let statements = self.parse_sql_statements(sql).expect(sql);
+ assert_eq!(statements.len(), statement_count);
Review Comment:
this assertion seems to already be covered by the if/else below? so that we
can skip the statement_count argument requirement?
##########
src/parser/mod.rs:
##########
@@ -4453,6 +4487,9 @@ impl<'a> Parser<'a> {
break;
}
}
+ if let Token::EOF = self.peek_nth_token_ref(0).token {
+ break;
+ }
Review Comment:
can we collapse this into above to use a match statement?
```rust
match &self.peek_nth_token_ref(0).token {
Token::Word(n) if ...
Token::Eof
}
```
##########
src/ast/mod.rs:
##########
@@ -3403,6 +3447,10 @@ pub enum Statement {
/// Cursor name
name: Ident,
direction: FetchDirection,
+ /// Differentiate between dialects that fetch `FROM` vs fetch `IN`
+ ///
+ ///
[MsSql](https://learn.microsoft.com/en-us/sql/t-sql/language-elements/fetch-transact-sql)
+ from_or_in: AttachedToken,
Review Comment:
Could we represent it with an explicit enum?
e.g
```rust
enum FetchPosition {
From
In
}
```
##########
tests/sqlparser_mssql.rs:
##########
@@ -1393,6 +1393,52 @@ fn parse_mssql_declare() {
let _ = ms().verified_stmt(declare_cursor_for_select);
}
+#[test]
+fn test_mssql_cursor() {
+ let full_cursor_usage = "\
+ DECLARE Employee_Cursor CURSOR FOR \
+ SELECT LastName, FirstName \
+ FROM AdventureWorks2022.HumanResources.vEmployee \
+ WHERE LastName LIKE 'B%'; \
+ \
+ OPEN Employee_Cursor; \
+ \
+ FETCH NEXT FROM Employee_Cursor; \
+ \
+ WHILE @@FETCH_STATUS = 0 \
+ BEGIN \
+ FETCH NEXT FROM Employee_Cursor; \
+ END; \
+ \
+ CLOSE Employee_Cursor; \
+ DEALLOCATE Employee_Cursor\
+ ";
+ let _ = ms().statements_parse_to(full_cursor_usage, 6, "");
+}
+
+#[test]
+fn test_mssql_while_statement() {
+ let while_single_statement = "WHILE 1 = 0 PRINT 'Hello World';";
Review Comment:
Can we include a test case with multiple statements in the while block?
Also since this introduces a new statement, can we include a test case that
asserts the returned AST?
##########
src/parser/mod.rs:
##########
@@ -8735,6 +8779,14 @@ impl<'a> Parser<'a> {
})
}
+ /// Parse [Statement::Open]
Review Comment:
maybe I missed it, we seem to be lacking test cases for the open statement
feature?
##########
src/keywords.rs:
##########
@@ -1064,6 +1065,8 @@ pub const RESERVED_FOR_TABLE_ALIAS: &[Keyword] = &[
Keyword::SAMPLE,
Keyword::TABLESAMPLE,
Keyword::FROM,
+ // Reserved for SQL Server cursors
Review Comment:
```suggestion
```
I think we can skip the comment, in case the keyword is used in other
contexts in the future
##########
src/ast/mod.rs:
##########
@@ -3032,6 +3068,14 @@ pub enum Statement {
partition: Option<Box<Expr>>,
},
/// ```sql
+ /// OPEN cursor_name
+ /// ```
+ /// Opens a cursor.
+ Open {
Review Comment:
Could we wrap this new statement in a named struct?
--
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
