Skip to content

Cover Savepoint rollback-to scope-exit semantics - #584

Open
SRombauts wants to merge 1 commit into
masterfrom
savepoint-rollback-semantics
Open

SRombauts wants to merge 1 commit into
masterfrom
savepoint-rollback-semantics

Conversation

@SRombauts

Copy link
Copy Markdown
Owner

Summary

  • document and test that ROLLBACK TO leaves a savepoint active
  • verify that writes made after a manual rollbackTo() are still rolled back on scope exit
  • verify that writes made after rollbackTo() can still be explicitly committed with release()
  • clarify the destructor's catch-all cleanup comment

Rationale

A deep review of the mbRolledBack member introduced by #559 and removed by #576 found that it should not be restored.

SQLite's ROLLBACK TO rewinds to the savepoint but keeps that savepoint active. If mbRolledBack permanently caused the destructor to skip its final ROLLBACK TO, then changes made after a manual rollback would be committed by the destructor's RELEASE instead of rolled back.

The existing implementation—always ROLLBACK TO before RELEASE on scope exit—is therefore the correct RAII behavior. This PR locks that behavior in with regression coverage.

No ABI or public API change is made.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 99.715%. remained the same — savepoint-rollback-semantics into master

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants