iffyio commented on code in PR #1521:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1521#discussion_r1844917951
##########
src/parser/mod.rs:
##########
@@ -3538,15 +3538,35 @@ impl<'a> Parser<'a> {
}
}
+ /// Parse the comma of a comma-separated syntax element.
+ /// Returns true if there is a next element
+ fn is_parse_comma_separated_end(&mut self) -> bool {
+ //default implementation
Review Comment:
```suggestion
```
##########
tests/sqlparser_snowflake.rs:
##########
@@ -2783,6 +2783,14 @@ fn test_parentheses_overflow() {
}
#[test]
+
+fn test_nested_select_with_lateral_flatten() {
+ let sql = "SELECT a FROM b, LATERAL FLATTEN(input => events)";
+ let _ = snowflake().parse_sql_statements(&sql).unwrap();
+
+ let sql = "SELECT (SELECT a FROM b, LATERAL FLATTEN(input => events))";
+ let _ = snowflake().parse_sql_statements(&sql).unwrap();
Review Comment:
hmm, the test function looks incomplete, missing a closing `}`? unless
github is masking that line somehow :thinking:
##########
src/parser/mod.rs:
##########
@@ -3481,13 +3481,12 @@ impl<'a> Parser<'a> {
//
https://cloud.google.com/bigquery/docs/reference/standard-sql/lexical#trailing_commas
//
https://docs.snowflake.com/en/release-notes/2024/8_11#select-supports-trailing-commas
//
Review Comment:
```suggestion
```
##########
tests/sqlparser_snowflake.rs:
##########
@@ -2783,6 +2783,14 @@ fn test_parentheses_overflow() {
}
#[test]
+
+fn test_nested_select_with_lateral_flatten() {
Review Comment:
Since the functionality doesnt seem to be related to the lateral flatten,
can we rename the test to e.g. `test_projection_with_nested_trailing_commas`?
And we can include a few more cases (e.g. doubly nested, multiple set
expressions that have trailing commas)?
##########
src/parser/mod.rs:
##########
@@ -3538,15 +3538,35 @@ impl<'a> Parser<'a> {
}
}
+ /// Parse the comma of a comma-separated syntax element.
+ /// Returns true if there is a next element
+ fn is_parse_comma_separated_end(&mut self) -> bool {
+ //default implementation
+
self.is_parse_comma_separated_end_with_trailing(self.options.trailing_commas)
+ }
+
/// Parse a comma-separated list of 1+ items accepted by `F`
- pub fn parse_comma_separated<T, F>(&mut self, mut f: F) -> Result<Vec<T>,
ParserError>
+ pub fn parse_comma_separated<T, F>(&mut self, f: F) -> Result<Vec<T>,
ParserError>
+ where
+ F: FnMut(&mut Parser<'a>) -> Result<T, ParserError>,
+ {
+ self.parse_comma_separated_with_trailing(f,
self.options.trailing_commas)
+ }
+
+ /// Parse a comma-separated list of 1+ items accepted by `F`
+ /// Allows for control over trailing commas
+ pub fn parse_comma_separated_with_trailing<T, F>(
Review Comment:
```suggestion
fn parse_comma_separated_with_trailing_commas<T, F>(
```
##########
src/parser/mod.rs:
##########
@@ -3516,11 +3515,12 @@ impl<'a> Parser<'a> {
}
/// Parse the comma of a comma-separated syntax element.
+ /// Allows for control over trailing commas
/// Returns true if there is a next element
- fn is_parse_comma_separated_end(&mut self) -> bool {
+ fn is_parse_comma_separated_end_with_trailing(&mut self, trailing_commas:
bool) -> bool {
Review Comment:
```suggestion
fn is_parse_comma_separated_end_with_trailing_commas(&mut self,
trailing_commas: bool) -> bool {
```
--
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
