You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "aharpervc (via GitHub)" <gi...@apache.org> on 2025/05/07 15:47:33 UTC
[PR] Add support for parsing with semicolons optional [datafusion-sqlparser-rs]
aharpervc opened a new pull request, #1843:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1843
Note: this PR is temporarily rebased on https://github.com/apache/datafusion-sqlparser-rs/pull/1834 before that is merged
---
This PR introduces support for parsing SQL without requiring semicolon statement delimiters. The implementation is as follows:
1. new ParserOption `require_semicolon_statement_delimiter` that defaults to a new Dialect function `supports_statements_without_semicolon_delimiter`
2. semicolon statement delimiters remain required by default for all dialects except SQL Server, where they're optional
To make this work, the following supplementary changes were required:
1. `RETURN` statement parsing was tightened up. I think I suggested in the prior PR for that function that it might need attention in the future (turns out the future is now). The reason this needed work is that without a semicolon to terminate the statement, it's hard to know if the thing following RETURN is part of RETURN or it's own thing. That ambiguity is addressed by having parse_return only allow a strict list of "returnable expr's" instead of any expr. This could have been it's own stand-alone PR but seems reasonable to review together instead of having a deeper stack of branches.
2. There are many tests that assert a parse statement error in the form of `Expected: end of statement, found: asdf`. When semicolons are optional, the parse error becomes `Expected: an SQL statement, found: asdf`. Since this is otherwise an acceptable error for that kind of test, a new helper was introduced `assert_err_parse_statements` that splits the dialects based on semicolon optional/required and asserts the appropriate error. The helper also has a pleasant side effect of reducing the volume of extremely similar error assertions.
3. Additionally for testing, a `statements_without_semicolons_parse_to` helper was added. It's the same as `statements_parse_to` but it will strip semicolons from the SQL text. This makes it easier to add supplementary assertions for both SQL variants.
4. Finally, I added a `test_supports_statements_without_semicolon_delimiter` test case to the SQL Server tests, to assert the proper AST for several examples not using semicolons. This is a belt and suspenders test to validate that parsing still produces the expected tree even without semicolons.
---
Fixes https://github.com/apache/datafusion-sqlparser-rs/issues/1800
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add support for parsing with semicolons optional [datafusion-sqlparser-rs]
Posted by "aharpervc (via GitHub)" <gi...@apache.org>.
aharpervc closed pull request #1843: Add support for parsing with semicolons optional
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1843
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add support for parsing with semicolons optional [datafusion-sqlparser-rs]
Posted by "aharpervc (via GitHub)" <gi...@apache.org>.
aharpervc commented on code in PR #1843:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1843#discussion_r2080184176
##########
tests/sqlparser_common.rs:
##########
@@ -666,6 +666,23 @@ fn parse_select_with_table_alias() {
);
}
+#[test]
+fn parse_consecutive_queries() {
Review Comment:
It turns out doing that seems reasonable and reduces ambiguity. Dialects that require semicolon delimiters remain unaffected, but dialects where it's optional would need to quote the keyword as an identifier. That seems completely reasonable.
##########
tests/sqlparser_common.rs:
##########
@@ -666,6 +666,23 @@ fn parse_select_with_table_alias() {
);
}
+#[test]
+fn parse_consecutive_queries() {
Review Comment:
It turns out doing that seems reasonable and reduces ambiguity. Dialects that require semicolon delimiters remain unaffected, but dialects where it's optional would need to quote the keyword as an identifier. That seems completely reasonable from a parser perspective.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add support for parsing with semicolons optional [datafusion-sqlparser-rs]
Posted by "aharpervc (via GitHub)" <gi...@apache.org>.
aharpervc commented on code in PR #1843:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1843#discussion_r2078393308
##########
tests/sqlparser_common.rs:
##########
@@ -666,6 +666,23 @@ fn parse_select_with_table_alias() {
);
}
+#[test]
+fn parse_consecutive_queries() {
Review Comment:
Not sure what the best approach here is, or if there's even a better solution apart from disallowing _any_ unquoted keyword as an alias
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add support for parsing with semicolons optional [datafusion-sqlparser-rs]
Posted by "aharpervc (via GitHub)" <gi...@apache.org>.
aharpervc commented on code in PR #1843:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1843#discussion_r2080184176
##########
tests/sqlparser_common.rs:
##########
@@ -666,6 +666,23 @@ fn parse_select_with_table_alias() {
);
}
+#[test]
+fn parse_consecutive_queries() {
Review Comment:
It turns out doing that seems reasonable and reduces ambiguity. Dialects that require semicolon delimiters remain unaffected, but dialects where it's optional would need to quote the keyword as an identifier. That seems completely reasonable from a parser perspective.
I'm willing to drop the commit that added keywords to RESERVED_FOR_TABLE_ALIAS since this approach addresses that concern.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add support for parsing with semicolons optional [datafusion-sqlparser-rs]
Posted by "aharpervc (via GitHub)" <gi...@apache.org>.
aharpervc commented on PR #1843:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1843#issuecomment-2859830502
Keeping this in draft at the moment to work out the "compile-no-std" ci job, and possibly some more test cases. However, anyone feel free to post a review/thoughts
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add support for parsing with semicolons optional [datafusion-sqlparser-rs]
Posted by "aharpervc (via GitHub)" <gi...@apache.org>.
aharpervc commented on PR #1843:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1843#issuecomment-3069973254
Looks like https://github.com/apache/datafusion-sqlparser-rs/pull/1937 introduced the parser option, so I'll close this PR rather than rebase. However, this PR also added some tests missing from that one. If necessary I'll make a separate PR for those tests.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org