Test Savepoint rollback semantics

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
 

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.
         }
     }
 }

tests/Savepoint_test.cpp

@@ -18,7 +18,7 @@
 
 #include <cstdio>
 
-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());