-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Rollback on constraint failure #1071
Rollback on constraint failure #1071
Conversation
6b59a48
to
7b40448
Compare
@rittneje Do you have an opinion? |
Given that database/sql will not allow a transaction to be reused once you call
|
Sounds good to me @rittneje. I've updated the PR and also updated the comment. |
sqlite3.go
Outdated
// However, database/sql considers the transaction complete once we | ||
// return from Commit() - we must clean up to honour its semantics. | ||
// We don't know if the ROLLBACK is strictly necessary, but according | ||
// to sqlite's docs, there is no hard in calling ROLLBACK unnecessarily. |
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.
// to sqlite's docs, there is no hard in calling ROLLBACK unnecessarily. | |
// to sqlite's docs, there is no harm in calling ROLLBACK unnecessarily. |
@joshbuddy LGTM, just one typo. |
75abef4
to
d07c0fc
Compare
@rittneje fixed, thank you |
Looks like some test flakiness? |
@joshbuddy I think there may be something wrong with for rows.Next() {
...
}
if err := rows.Err(); err != nil {
t.Fatal(err)
}
if n != 3 {
...
} |
Hey @rittneje, I fixed the underlying test. If you want me to split it off into a separate PR, let me know. |
e2bf557
to
15d5547
Compare
I've moved the test fixes to #1079 |
15d5547
to
0f9d82e
Compare
Codecov Report
@@ Coverage Diff @@
## master #1071 +/- ##
=======================================
Coverage 45.96% 45.96%
=======================================
Files 11 11
Lines 1499 1499
=======================================
Hits 689 689
Misses 670 670
Partials 140 140
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Always rollback on a commit error
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [github.com/mattn/go-sqlite3](https://togithub.com/mattn/go-sqlite3) | require | patch | `v1.14.15` -> `v1.14.16` | --- ### Release Notes <details> <summary>mattn/go-sqlite3</summary> ### [`v1.14.16`](https://togithub.com/mattn/go-sqlite3/releases/tag/v1.14.16) [Compare Source](https://togithub.com/mattn/go-sqlite3/compare/v1.14.15...v1.14.16) #### What's Changed - Add build tag to enable OSTRACE() logging by [@​benbjohnson](https://togithub.com/benbjohnson) in [https://github.com/mattn/go-sqlite3/pull/1067](https://togithub.com/mattn/go-sqlite3/pull/1067) - TestQueryer: actually check Rows returned by [@​ohwgiles](https://togithub.com/ohwgiles) in [https://github.com/mattn/go-sqlite3/pull/1062](https://togithub.com/mattn/go-sqlite3/pull/1062) - Update README to fix reference URL by [@​shibukawa](https://togithub.com/shibukawa) in [https://github.com/mattn/go-sqlite3/pull/1082](https://togithub.com/mattn/go-sqlite3/pull/1082) - Fix test queryer test by [@​joshbuddy](https://togithub.com/joshbuddy) in [https://github.com/mattn/go-sqlite3/pull/1079](https://togithub.com/mattn/go-sqlite3/pull/1079) - Rollback on constraint failure by [@​joshbuddy](https://togithub.com/joshbuddy) in [https://github.com/mattn/go-sqlite3/pull/1071](https://togithub.com/mattn/go-sqlite3/pull/1071) - Fix "ennviroment" by [@​RewardedIvan](https://togithub.com/RewardedIvan) in [https://github.com/mattn/go-sqlite3/pull/1077](https://togithub.com/mattn/go-sqlite3/pull/1077) - こんにちわ is wrong Japanse, Correct word is こんにちは by [@​KiYugadgeter](https://togithub.com/KiYugadgeter) in [https://github.com/mattn/go-sqlite3/pull/1085](https://togithub.com/mattn/go-sqlite3/pull/1085) - Add test for sqlite_math_functions tag by [@​lggruspe](https://togithub.com/lggruspe) in [https://github.com/mattn/go-sqlite3/pull/1059](https://togithub.com/mattn/go-sqlite3/pull/1059) - remove unuseful ldflags for windows platform. by [@​kkqy](https://togithub.com/kkqy) in [https://github.com/mattn/go-sqlite3/pull/1084](https://togithub.com/mattn/go-sqlite3/pull/1084) - Cross Compiling for Mac OS via `musl-cross` by [@​jodosha](https://togithub.com/jodosha) in [https://github.com/mattn/go-sqlite3/pull/1090](https://togithub.com/mattn/go-sqlite3/pull/1090) - Update README.md to include vtable feature by [@​dvas0004](https://togithub.com/dvas0004) in [https://github.com/mattn/go-sqlite3/pull/1100](https://togithub.com/mattn/go-sqlite3/pull/1100) - Updating vtable example, "BestIndex" method by [@​dvas0004](https://togithub.com/dvas0004) in [https://github.com/mattn/go-sqlite3/pull/1099](https://togithub.com/mattn/go-sqlite3/pull/1099) - Update amalgamation code by [@​mattn](https://togithub.com/mattn) in [https://github.com/mattn/go-sqlite3/pull/1104](https://togithub.com/mattn/go-sqlite3/pull/1104) #### New Contributors - [@​ohwgiles](https://togithub.com/ohwgiles) made their first contribution in [https://github.com/mattn/go-sqlite3/pull/1062](https://togithub.com/mattn/go-sqlite3/pull/1062) - [@​shibukawa](https://togithub.com/shibukawa) made their first contribution in [https://github.com/mattn/go-sqlite3/pull/1082](https://togithub.com/mattn/go-sqlite3/pull/1082) - [@​joshbuddy](https://togithub.com/joshbuddy) made their first contribution in [https://github.com/mattn/go-sqlite3/pull/1079](https://togithub.com/mattn/go-sqlite3/pull/1079) - [@​RewardedIvan](https://togithub.com/RewardedIvan) made their first contribution in [https://github.com/mattn/go-sqlite3/pull/1077](https://togithub.com/mattn/go-sqlite3/pull/1077) - [@​KiYugadgeter](https://togithub.com/KiYugadgeter) made their first contribution in [https://github.com/mattn/go-sqlite3/pull/1085](https://togithub.com/mattn/go-sqlite3/pull/1085) - [@​lggruspe](https://togithub.com/lggruspe) made their first contribution in [https://github.com/mattn/go-sqlite3/pull/1059](https://togithub.com/mattn/go-sqlite3/pull/1059) - [@​kkqy](https://togithub.com/kkqy) made their first contribution in [https://github.com/mattn/go-sqlite3/pull/1084](https://togithub.com/mattn/go-sqlite3/pull/1084) - [@​jodosha](https://togithub.com/jodosha) made their first contribution in [https://github.com/mattn/go-sqlite3/pull/1090](https://togithub.com/mattn/go-sqlite3/pull/1090) - [@​dvas0004](https://togithub.com/dvas0004) made their first contribution in [https://github.com/mattn/go-sqlite3/pull/1100](https://togithub.com/mattn/go-sqlite3/pull/1100) **Full Changelog**: mattn/go-sqlite3@v1.14.15...v1.14.16 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/mergestat/mergestat). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xLjUiLCJ1cGRhdGVkSW5WZXIiOiIzNC4xLjUifQ==-->
This fixes an issue where deferred constraints can leave the transaction in an inconsistent state. In the event of a failed deferred constraint, we need to issue a rollback in order to make a transaction startable later.