Incorrect work of savepoint

[spoyler]

Hi.
I'm using savepoint, but it behavior is not what I expected.
If savepoint is used as code below, everything works is good, data would be saved into database.

SQLite::Savepoint save_point(*db_, "savepoint1");
SQLite::Statement query(*db_,"UPDATE compositions SET latitude = ?, longitude = ? WHERE id = ?");
query.bind(1, geolocation.latitude);
query.bind(2, geolocation.longitude);
query.bind(3, composition_id);
query.exec();
save_point.release();

But, if I call rollback(), or savepoint call rollback() from destructor, then transaction is not canceled, it would be restarting from beginning. And if I made some other changes in database and close my application, data will not be saved, because savepoint is still active, and it not released.

SQlite documentation say about this:

The ROLLBACK TO command reverts the state of the database back to what it was just after the corresponding SAVEPOINT. >Note that unlike that plain ROLLBACK command (without the TO keyword) the >ROLLBACK TO command does not cancel the transaction. Instead of cancelling the transaction, the ROLLBACK TO command >restarts the transaction again at the beginning. All intervening SAVEPOINTs are canceled, however.

I think, need call RELEASE savepoint command after ROLLBACK for removing savepoint from transaction stack.

[SRombauts]

Thanks for reporting the issue!
I don't think that I understand the problem correctly, it would nee me to take some time to read and then reproduce... do you have a change in mind? a patch or a PR?

[spoyler]

I have created a pull request to fix this problem.
#390

[spoyler]

You can reproduce problem with this test:

#include <sqlite3.h>

#include <filesystem>
#include <memory>

#include <SQLiteCpp/SQLiteCpp.h>
#include <SQLiteCpp/Savepoint.h>
#include <SQLiteCpp/VariadicBind.h>
#include <gtest/gtest.h>

static const auto kConfigStorageTestDir = std::filesystem::temp_directory_path() / "gtest";

static constexpr std::string_view kDbTestName = "TestDb.sqlite3";
static constexpr std::string_view kCompositionName = "CompositionTestName";
static constexpr std::string_view kCompositionType = "CompositionTestType";

static constexpr auto kCreateCompositionsTable = R"SQL(
CREATE TABLE "compositions" (
	"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
	"name" TEXT NOT NULL,
	"type" TEXT NOT NULL,
	"latitude" REAL,
	"longitude" REAL,
	"altitude" REAL,
	"map_id" INTEGER)
)SQL";

class SavepointFixture : public ::testing::Test {
protected:
	void SetUp() override {
		std::filesystem::remove_all(kConfigStorageTestDir);
		std::filesystem::create_directories(kConfigStorageTestDir);
		db_path_ = kConfigStorageTestDir / kDbTestName;
		db_.emplace(db_path_, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE);
		db_->exec("PRAGMA journal_mode = WAL");
		db_->exec(kCreateCompositionsTable);
	}

	void TearDown() override {
		db_.reset();
		std::filesystem::remove_all(kConfigStorageTestDir);
	}

	SQLite::Database &GetDataBase() {
		return *db_;
	}

	bool ReopenDataBase() {
		if (!exists(std::filesystem::path(db_path_)))
			return false;

		db_.reset();
		db_.emplace(db_path_, SQLITE_OPEN_READWRITE);
		db_->exec("PRAGMA journal_mode = WAL");
		return true;
	}

private:
	std::filesystem::path db_path_;
	std::optional<SQLite::Database> db_;
};

TEST_F(SavepointFixture, CreateSavePoint_InsertIntoTable_ReadDataSuccess) {
	{
		SQLite::Savepoint savepoint(GetDataBase(), "first");
		savepoint.rollback();
	}
	SQLite::Statement query(GetDataBase(), "INSERT INTO compositions VALUES (NULL, ?, ? , NULL, NULL, NULL, NULL)");
	SQLite::bind(query, kCompositionName.data(), kCompositionType.data());
	query.exec();
	const int64_t composition_id = GetDataBase().getLastInsertRowid();
	ASSERT_TRUE(ReopenDataBase());

	SQLite::Statement sel(GetDataBase(), "SELECT name, type FROM compositions WHERE id = ?");
	SQLite::bind(sel, composition_id);
	ASSERT_TRUE(sel.executeStep());

	ASSERT_TRUE(kCompositionName == sel.getColumn(0).getText());
	ASSERT_TRUE(kCompositionType == sel.getColumn(1).getText());
}

[SRombauts]

Hello,

class SavepointFixture : public ::testing::Test {
protected:
}

I don't understand how this test can show issues with savepoints; the unit test won't fail, right?
How can you see that the savepoint "first" is not correctly released from that?

[spoyler]

Hello,
I don't understand this quote about the test

class SavepointFixture : public ::testing::Test {
protected:
}

Savepoint "first" is not correctly released in this place:

{
  SQLite::Savepoint savepoint(GetDataBase(), "first");
  savepoint.rollback();
}

After rollback method was executed and savepoint will go out of scope, it still present in database transaction stack.
And when the database would be closed ASSERT_TRUE(ReopenDataBase());, all changes made after savepoint "first" would be lost.

I can't add this test as is, because it use C++17, but minimal required version for SQLiteCpp is C++11.
And I need some free time to rewrite it.