-
Notifications
You must be signed in to change notification settings - Fork 597
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
Support "UPDATE" statement in "WITH" subquery #842
Conversation
Thanks so much for the contribution @nicksrandall! Could you add some tests that demonstrate the new behavior? |
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.
Looks good to me, although the test could still be a bit better.
@alamb will leave it up to you on whether to push on that!
|
||
#[test] | ||
fn test_update_in_with_subquery() { | ||
let sql = r#"WITH "result" AS (UPDATE "Hero" SET "name" = 'Captain America', "number_of_movies" = "number_of_movies" + 1 WHERE "secret_identity" = 'Sam Wilson' RETURNING "id", "name", "secret_identity", "number_of_movies") SELECT * FROM "result""#; |
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.
Generally it's best to test across dialects, using the testing helpers (e.g. see test_parse_limit
earlier in the file). In this case, the change itself has little to do with the dialect itself, so I'm not particularly concerned by it.
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.
Yeah, I agree the tests are a little out of place here -- in general it seems like we have some tests in parser.rs
that really belong in src/tests/...
I'll move them around in a follow on PR
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.
Pull Request Test Coverage Report for Build 4534413969
💛 - Coveralls |
Thanks @nicksrandall and @ankrgyl |
I was getting an error trying to parse this query with postgres: