-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Upgrade to sqlparser 0.61.0 #20177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Upgrade to sqlparser 0.61.0 #20177
Conversation
|
There are a few sqllogictest errors like this |
c95affc to
93fa56e
Compare
|
I had Codex make up a DataFusion specific dialect to get the tests passing. I now need to clean it up, Occam's razor style |
|
Should we perhaps consider the errors as bugs in |
|
I have an issue and a PR for this upstream: apache/datafusion-sqlparser-rs#2204 and apache/datafusion-sqlparser-rs#2205 Seems to fix all the 35 errors here |
Yes, likely -- this is great news -- thank you @Samyak2 |
93fa56e to
8bda485
Compare
8bda485 to
315e7c0
Compare
315e7c0 to
0461c5a
Compare
| }) => { | ||
| if temporary { | ||
| return not_impl_err!("Temporary tables not supported")?; | ||
| return not_impl_err!("Temporary tables not supported"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a drive by cleanup -- no need to return and ? -- just return is fine
Thanks again @Samyak2 -- I updated this branch to include the fix (and the 0.61.0 Release candidate) and all the tests pass now. Thanks 🙏 ! |
DRAFT until SQL parser is released
Which issue does this PR close?
0.61.0around 2026-02-01 datafusion-sqlparser-rs#2117Rationale for this change
Keep up to date with dependencies
I think @Samyak2 specifically would like access to the
:field syntaxWhat changes are included in this PR?
Are these changes tested?
Yes by existing tests
Are there any user-facing changes?
New dependency