Call GetActiveSnapshot when creating TX (#246)

Co-authored-by: Jelte Fennema-Nio <[email protected]>
duckdb · Oct 4, 2024 · 01f3050 · 01f3050
1 parent aa3c27c
commit 01f3050
Show file tree

Hide file tree

Showing 8 changed files with 26 additions and 23 deletions.
diff --git a/include/pgduckdb/catalog/pgduckdb_storage.hpp b/include/pgduckdb/catalog/pgduckdb_storage.hpp
@@ -10,18 +10,9 @@ extern "C" {
 
 namespace duckdb {
 
-class PostgresStorageExtensionInfo : public StorageExtensionInfo {
-public:
-	PostgresStorageExtensionInfo(Snapshot snapshot) : snapshot(snapshot) {
-	}
-
-public:
-	Snapshot snapshot;
-};
-
 class PostgresStorageExtension : public StorageExtension {
 public:
-	PostgresStorageExtension(Snapshot snapshot);
+	PostgresStorageExtension();
 };
 
 } // namespace duckdb
diff --git a/include/pgduckdb/catalog/pgduckdb_transaction_manager.hpp b/include/pgduckdb/catalog/pgduckdb_transaction_manager.hpp
@@ -9,7 +9,7 @@ namespace duckdb {
 
 class PostgresTransactionManager : public TransactionManager {
 public:
-	PostgresTransactionManager(AttachedDatabase &db_p, PostgresCatalog &catalog, Snapshot snapshot);
+	PostgresTransactionManager(AttachedDatabase &db_p, PostgresCatalog &catalog);
 
 	Transaction &StartTransaction(ClientContext &context) override;
 	ErrorData CommitTransaction(ClientContext &context, Transaction &transaction) override;
@@ -19,7 +19,6 @@ class PostgresTransactionManager : public TransactionManager {
 
 private:
 	PostgresCatalog &catalog;
-	Snapshot snapshot;
 	mutex transaction_lock;
 	reference_map_t<Transaction, unique_ptr<Transaction>> transactions;
 };

diff --git a/src/catalog/pgduckdb_storage.cpp b/src/catalog/pgduckdb_storage.cpp
@@ -6,16 +6,12 @@ namespace duckdb {
 
 static unique_ptr<TransactionManager>
 CreateTransactionManager(StorageExtensionInfo *storage_info, AttachedDatabase &db, Catalog &catalog) {
-	auto &pg_storage_info = *(reinterpret_cast<PostgresStorageExtensionInfo *>(storage_info));
-	auto snapshot = pg_storage_info.snapshot;
-
-	return make_uniq<PostgresTransactionManager>(db, catalog.Cast<PostgresCatalog>(), snapshot);
+	return make_uniq<PostgresTransactionManager>(db, catalog.Cast<PostgresCatalog>());
 }
 
-PostgresStorageExtension::PostgresStorageExtension(Snapshot snapshot) {
+PostgresStorageExtension::PostgresStorageExtension() {
 	attach = PostgresCatalog::Attach;
 	create_transaction_manager = CreateTransactionManager;
-	storage_info = make_uniq<PostgresStorageExtensionInfo>(snapshot);
 }
 
 } // namespace duckdb
diff --git a/src/catalog/pgduckdb_transaction_manager.cpp b/src/catalog/pgduckdb_transaction_manager.cpp
@@ -1,16 +1,16 @@
 #include "pgduckdb/catalog/pgduckdb_transaction_manager.hpp"
 #include "duckdb/main/attached_database.hpp"
+#include "pgduckdb/pgduckdb_process_lock.hpp"
 
 namespace duckdb {
 
-PostgresTransactionManager::PostgresTransactionManager(AttachedDatabase &db_p, PostgresCatalog &catalog,
-                                                       Snapshot snapshot)
-    : TransactionManager(db_p), catalog(catalog), snapshot(snapshot) {
+PostgresTransactionManager::PostgresTransactionManager(AttachedDatabase &db_p, PostgresCatalog &catalog)
+    : TransactionManager(db_p), catalog(catalog) {
 }
 
 Transaction &
 PostgresTransactionManager::StartTransaction(ClientContext &context) {
-	auto transaction = make_uniq<PostgresTransaction>(*this, context, catalog, snapshot);
+	auto transaction = make_uniq<PostgresTransaction>(*this, context, catalog, GetActiveSnapshot());
 	auto &result = *transaction;
 	lock_guard<mutex> l(transaction_lock);
 	transactions[result] = std::move(transaction);

diff --git a/src/pgduckdb_duckdb.cpp b/src/pgduckdb_duckdb.cpp
@@ -80,7 +80,7 @@ DuckDBManager::DuckDBManager() {
 
 	database = duckdb::make_uniq<duckdb::DuckDB>(nullptr, &config);
 	duckdb::DBConfig::GetConfig(*database->instance).storage_extensions["pgduckdb"] =
-	    duckdb::make_uniq<duckdb::PostgresStorageExtension>(GetActiveSnapshot());
+	    duckdb::make_uniq<duckdb::PostgresStorageExtension>();
 	duckdb::ExtensionInstallInfo extension_install_info;
 	database->instance->SetExtensionLoaded("pgduckdb", extension_install_info);
 

diff --git a/test/regression/expected/transaction_errors.out b/test/regression/expected/transaction_errors.out
@@ -0,0 +1,12 @@
+CREATE TABLE foo AS SELECT 'bar' AS t;
+BEGIN; SET duckdb.execution = true; SELECT t::integer AS t1 FROM foo; ROLLBACK;
+ERROR:  (PGDuckDB/ExecuteQuery) Conversion Error: Could not convert string 'bar' to INT32
+LINE 1: SELECT (t)::integer AS t1 FROM public.foo
+                  ^
+SET duckdb.execution = true;
+SELECT 1 FROM foo;
+ ?column? 
+----------
+        1
+(1 row)
+
diff --git a/test/regression/schedule b/test/regression/schedule
@@ -14,3 +14,4 @@ test: create_table_as
 test: standard_conforming_strings
 test: query_filter
 test: altered_tables
+test: transaction_errors
diff --git a/test/regression/sql/transaction_errors.sql b/test/regression/sql/transaction_errors.sql
@@ -0,0 +1,4 @@
+CREATE TABLE foo AS SELECT 'bar' AS t;
+BEGIN; SET duckdb.execution = true; SELECT t::integer AS t1 FROM foo; ROLLBACK;
+SET duckdb.execution = true;
+SELECT 1 FROM foo;