-
Notifications
You must be signed in to change notification settings - Fork 669
delete: add using clause, possibility of using aliases #541
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
Conversation
Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com>
Pull Request Test Coverage Report for Build 2671290535
💛 - Coveralls |
alamb
left a comment
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.
Thank you @mobuchowski -- looking good. I think this PR just needs one more test
| table_name, | ||
| ); | ||
|
|
||
| assert_eq!(None, using); |
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.
Can you please add a positive test for using as well (aka a query that has a USING clause)?
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.
@alamb I added it few lines below already: https://github.com/sqlparser-rs/sqlparser-rs/pull/541/files#diff-ae2dab2c924f4018c1daf0743b8fd912c44c5b63ae7144dd8b951bb3bd254349R305
Unless you mean something else? Like, more complex TableFactor?
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.
No this is fine -- I just wanted to add some coverage so that we didn't accidentally break the functionality in the future but have all the tests pass
| table_name, | ||
| ); | ||
|
|
||
| assert_eq!(None, using); |
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.
No this is fine -- I just wanted to add some coverage so that we didn't accidentally break the functionality in the future but have all the tests pass
|
Thanks again @mobuchowski |
Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com>
PostgreSQL and MySQL have possibility of using aliases in
DELETEstatements.PostgreSQL and Snowflake have
USINGclause.Signed-off-by: Maciej Obuchowski obuchowski.maciej@gmail.com