From 7d30953fbbf862e995a8fade71faf0b3f042f06b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Rombauts?= Date: Wed, 23 Sep 2026 11:30:34 +0200 Subject: [PATCH] Test Savepoint rollback semantics --- CHANGELOG.md | 1 + src/Savepoint.cpp | 3 +- tests/Savepoint_test.cpp | 81 +++++++++++++++++++++++++++++++--------- 3 files changed, 66 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 394a7a77..433f18d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump the shared-library ABI version from 0 to 1 for SQLiteCpp 4.0 - Use fixed-width integer types in `SQLite::Header`, making field widths consistent across platforms (ABI change) (#582) - Update googletest to v1.18.0 (#583) +- Improve tests for `Savepoint::rollbackTo()` and scope-exit rollback semantics (#584) ## [3.4.0] - 2026-09-21 diff --git a/src/Savepoint.cpp b/src/Savepoint.cpp index 36cf36aa..704cc4cd 100644 --- a/src/Savepoint.cpp +++ b/src/Savepoint.cpp @@ -45,8 +45,7 @@ Savepoint::~Savepoint() } catch (...) { - // Never throw an exception in a destructor: error if already released, - // but no harm is caused by this. + // Never let best-effort rollback/release cleanup throw from the destructor. } } } diff --git a/tests/Savepoint_test.cpp b/tests/Savepoint_test.cpp index a5c39423..c55004d6 100644 --- a/tests/Savepoint_test.cpp +++ b/tests/Savepoint_test.cpp @@ -18,7 +18,7 @@ #include -TEST(Savepoint, commitRollback) +TEST(Savepoint, releaseAndRollback) { // Create a new database SQLite::Database db(":memory:", SQLite::OPEN_READWRITE | SQLite::OPEN_CREATE); @@ -38,35 +38,35 @@ TEST(Savepoint, commitRollback) // release savepoint savepoint.release(); - // Commit again throw an exception + // Releasing or rolling back an already released savepoint throws. EXPECT_THROW(savepoint.release(), SQLite::Exception); EXPECT_THROW(savepoint.rollback(), SQLite::Exception); } - // Auto rollback if no release() before the end of scope + // Automatic rollback if release() is not called before the end of scope. { // Begin savepoint SQLite::Savepoint savepoint(db, "sp2"); - // Insert a second value (that will be rollbacked) + // Insert a second value that will be rolled back. EXPECT_EQ(1, db.exec("INSERT INTO test VALUES (NULL, 'third')")); EXPECT_EQ(2, db.getLastInsertRowid()); // end of scope: automatic rollback } - // Auto rollback of a transaction on error / exception + // Automatic rollback of a savepoint when leaving scope because of an exception. try { // Begin savepoint SQLite::Savepoint savepoint(db, "sp3"); - // Insert a second value (that will be rollbacked) + // Insert a second value that will be rolled back. EXPECT_EQ(1, db.exec("INSERT INTO test VALUES (NULL, 'second')")); EXPECT_EQ(2, db.getLastInsertRowid()); - // Execute with an error => exception with auto-rollback - db.exec("DesiredSyntaxError to raise an exception to rollback the transaction"); + // Trigger an exception; stack unwinding destroys the savepoint and rolls it back. + db.exec("DesiredSyntaxError to raise an exception and rollback the savepoint"); GTEST_FATAL_FAILURE_("we should never get there"); savepoint.release(); // We should never get there @@ -77,23 +77,22 @@ TEST(Savepoint, commitRollback) // expected error, see above } - // Double rollback with a manual command before the end of scope + // Manual rollback before the end of scope { // Begin savepoint SQLite::Savepoint savepoint(db, "sp4"); - // Insert a second value (that will be rollbacked) + // Insert a second value (that will be rolled back) EXPECT_EQ(1, db.exec("INSERT INTO test VALUES (NULL, 'third')")); EXPECT_EQ(2, db.getLastInsertRowid()); - // Execute a manual rollback - savepoint.rollback(); + // Roll back everything done since sp4, while keeping the savepoint active. + savepoint.rollbackTo(); - // end of scope: the automatic rollback should not raise an error because it is harmless + // end of scope: normal automatic rollback/release cleanup } - // Check the results (expect only one row of result, as all other one have - // been rollbacked) + // Only the explicitly released first row should remain; all later rows were rolled back. SQLite::Statement query(db, "SELECT * FROM test"); int nbRows = 0; while (query.executeStep()) @@ -105,6 +104,27 @@ TEST(Savepoint, commitRollback) EXPECT_EQ(1, nbRows); } +TEST(Savepoint, rollbackTo) +{ + SQLite::Database db(":memory:", SQLite::OPEN_READWRITE | SQLite::OPEN_CREATE); + db.exec("CREATE TABLE test (id INTEGER PRIMARY KEY, value TEXT)"); + + SQLite::Savepoint savepoint(db, "sp"); + + EXPECT_EQ(1, db.exec("INSERT INTO test VALUES (NULL, 'first')")); + EXPECT_EQ(1, db.exec("INSERT INTO test VALUES (NULL, 'second')")); + + savepoint.rollbackTo(); + + // ROLLBACK TO must undo all changes made since the savepoint was created. + SQLite::Statement query(db, "SELECT COUNT(*) FROM test"); + ASSERT_TRUE(query.executeStep()); + EXPECT_EQ(0, query.getColumn(0).getInt()); + + // ROLLBACK TO keeps the savepoint active, so release it explicitly. + EXPECT_NO_THROW(savepoint.release()); +} + TEST(Savepoint, rollbackToThenRelease) { // Create a new database @@ -117,14 +137,41 @@ TEST(Savepoint, rollbackToThenRelease) EXPECT_EQ(1, db.exec("INSERT INTO test VALUES (NULL, 'rolled back')")); - // A manual rollback leaves the savepoint on the stack, so releasing it afterwards succeeds. + // ROLLBACK TO leaves the savepoint active. New changes can still be made + // and explicitly committed by releasing the savepoint. savepoint.rollbackTo(); + EXPECT_EQ(1, db.exec("INSERT INTO test VALUES (NULL, 'kept')")); EXPECT_NO_THROW(savepoint.release()); // end of scope: already released, the destructor must do nothing and not throw } - // The rolled-back insert must not be persisted + // The pre-rollback row is gone; only the row added after ROLLBACK TO was committed. + SQLite::Statement count(db, "SELECT COUNT(*) FROM test"); + ASSERT_TRUE(count.executeStep()); + EXPECT_EQ(1, count.getColumn(0).getInt()); + + SQLite::Statement query(db, "SELECT value FROM test"); + ASSERT_TRUE(query.executeStep()); + EXPECT_STREQ("kept", query.getColumn(0).getText()); +} + +TEST(Savepoint, autoRollbackAfterManualRollbackTo) +{ + SQLite::Database db(":memory:", SQLite::OPEN_READWRITE | SQLite::OPEN_CREATE); + db.exec("CREATE TABLE test (id INTEGER PRIMARY KEY, value TEXT)"); + + { + SQLite::Savepoint savepoint(db, "sp"); + + EXPECT_EQ(1, db.exec("INSERT INTO test VALUES (NULL, 'before rollback')")); + savepoint.rollbackTo(); + + // ROLLBACK TO restarts the savepoint instead of ending it. Changes made + // afterwards must therefore still be rolled back on scope exit. + EXPECT_EQ(1, db.exec("INSERT INTO test VALUES (NULL, 'after rollback')")); + } + SQLite::Statement query(db, "SELECT COUNT(*) FROM test"); ASSERT_TRUE(query.executeStep()); EXPECT_EQ(0, query.getColumn(0).getInt());