-
Notifications
You must be signed in to change notification settings - Fork 163
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
Add merge completion case #1026
Add merge completion case #1026
Conversation
Thanks! Would you mind retargeting this to the series/0.6.x branch? |
d73e04f
to
6bd7f7a
Compare
Re-triggering CI ... |
I believe that the tests run on pg 11, while MERGE has been introduced in pg 15. Should we run the test conditionally? |
Can we make the test check for merge support and then skip if not enabled on the server? |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## series/0.6.x #1026 +/- ##
================================================
- Coverage 84.33% 84.29% -0.04%
================================================
Files 126 126
Lines 1736 1738 +2
Branches 197 180 -17
================================================
+ Hits 1464 1465 +1
- Misses 272 273 +1 ☔ View full report in Codecov by Sentry. |
} yield "ok") | ||
.recoverWith { | ||
case SqlState.SyntaxError(ex) if ex.message.startsWith("""Syntax error at or near "MERGE"""") => s.execute(deleteCity)(Garin.id).as("ok") | ||
} |
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.
Hm, instead of this can we do something like SELECT version();
to decide whether or not to run this test?
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.
Ah thanks! I was looking for querying direct support of merge, but that's much better of course.
It doesn't change the discussion above, but PG 11 seems EOL since November 2023 (https://www.postgresql.org/support/versioning/). Should we upgrade the docker to PG 12 (in a separate PR)? |
Yeah that's a good idea. |
No description provided.