From 0306842ed1084bdb31de5d81256477874d8c2ceb Mon Sep 17 00:00:00 2001 From: Mytherin Date: Sat, 14 Mar 2026 18:31:17 +0100 Subject: [PATCH 1/2] Make access to catalog entries in SQLiteTransaction thread-safe --- duckdb | 2 +- src/include/storage/sqlite_transaction.hpp | 3 +- src/storage/sqlite_transaction.cpp | 51 ++++++++++++++++++---- test/sql/storage/attach_sakila.test | 25 +++++++++++ 4 files changed, 71 insertions(+), 10 deletions(-) create mode 100644 test/sql/storage/attach_sakila.test diff --git a/duckdb b/duckdb index f480e78..c53f190 160000 --- a/duckdb +++ b/duckdb @@ -1 +1 @@ -Subproject commit f480e781694b03105c9281d2564f08adb9a8d4a8 +Subproject commit c53f19074c01a0e8a039785a8814799c163857cb diff --git a/src/include/storage/sqlite_transaction.hpp b/src/include/storage/sqlite_transaction.hpp index dd7095d..0a71689 100644 --- a/src/include/storage/sqlite_transaction.hpp +++ b/src/include/storage/sqlite_transaction.hpp @@ -15,6 +15,7 @@ namespace duckdb { class SQLiteCatalog; class SQLiteTableEntry; +class SQLiteCatalogMap; class SQLiteTransaction : public Transaction { public: @@ -36,7 +37,7 @@ class SQLiteTransaction : public Transaction { SQLiteCatalog &sqlite_catalog; SQLiteDB *db; SQLiteDB owned_db; - case_insensitive_map_t> catalog_entries; + unique_ptr catalog_map; }; } // namespace duckdb diff --git a/src/storage/sqlite_transaction.cpp b/src/storage/sqlite_transaction.cpp index 4dc0296..08ea220 100644 --- a/src/storage/sqlite_transaction.cpp +++ b/src/storage/sqlite_transaction.cpp @@ -15,6 +15,42 @@ namespace duckdb { +class SQLiteCatalogMap { +public: + optional_ptr InsertEntry(const string &entry_name, unique_ptr entry); + optional_ptr GetEntry(const string &entry_name); + void EraseEntry(const string &entry_name); +private: + mutex lock; + case_insensitive_map_t> catalog_entries; +}; + + +optional_ptr SQLiteCatalogMap::InsertEntry(const string &entry_name, unique_ptr catalog_entry) { + lock_guard guard(lock); + auto entry = catalog_entries.find(entry_name); + if (entry != catalog_entries.end()) { + return entry->second.get(); + } + auto &result = *catalog_entry; + catalog_entries[entry_name] = std::move(catalog_entry); + return result; +} + +optional_ptr SQLiteCatalogMap::GetEntry(const string &entry_name) { + lock_guard guard(lock); + auto entry = catalog_entries.find(entry_name); + if (entry != catalog_entries.end()) { + return entry->second.get(); + } + return nullptr; +} + +void SQLiteCatalogMap::EraseEntry(const string &entry_name) { + lock_guard guard(lock); + catalog_entries.erase(entry_name); +} + SQLiteTransaction::SQLiteTransaction(SQLiteCatalog &sqlite_catalog, TransactionManager &manager, ClientContext &context) : Transaction(manager, context), sqlite_catalog(sqlite_catalog) { if (sqlite_catalog.InMemory()) { @@ -25,6 +61,7 @@ SQLiteTransaction::SQLiteTransaction(SQLiteCatalog &sqlite_catalog, TransactionM owned_db = SQLiteDB::Open(sqlite_catalog.path, sqlite_catalog.options, true); db = &owned_db; } + catalog_map = make_uniq(); } SQLiteTransaction::~SQLiteTransaction() { @@ -106,9 +143,9 @@ unique_ptr FromCreateIndex(ClientContext &context, TableCatalog } optional_ptr SQLiteTransaction::GetCatalogEntry(const string &entry_name) { - auto entry = catalog_entries.find(entry_name); - if (entry != catalog_entries.end()) { - return entry->second.get(); + auto entry = catalog_map->GetEntry(entry_name); + if (entry) { + return entry; } // catalog entry not found - look up table in main SQLite database auto type = db->GetEntryType(entry_name); @@ -169,13 +206,11 @@ optional_ptr SQLiteTransaction::GetCatalogEntry(const string &entr default: throw InternalException("Unrecognized catalog entry type"); } - auto result_ptr = result.get(); - catalog_entries[entry_name] = std::move(result); - return result_ptr; + return catalog_map->InsertEntry(entry_name, std::move(result)); } void SQLiteTransaction::ClearTableEntry(const string &table_name) { - catalog_entries.erase(table_name); + catalog_map->EraseEntry(table_name); } string GetDropSQL(CatalogType type, const string &table_name, bool cascade) { @@ -199,7 +234,7 @@ string GetDropSQL(CatalogType type, const string &table_name, bool cascade) { } void SQLiteTransaction::DropEntry(CatalogType type, const string &table_name, bool cascade) { - catalog_entries.erase(table_name); + catalog_map->EraseEntry(table_name); db->Execute(GetDropSQL(type, table_name, cascade)); } diff --git a/test/sql/storage/attach_sakila.test b/test/sql/storage/attach_sakila.test new file mode 100644 index 0000000..5861c7e --- /dev/null +++ b/test/sql/storage/attach_sakila.test @@ -0,0 +1,25 @@ +# name: test/sql/scanner/sakila.test +# description: Test sakila database +# group: [sqlite_scanner] + +require sqlite_scanner + +statement ok +ATTACH 'data/db/sakila.db' (READ_ONLY); + +statement ok +USE sakila; + +statement ok +SELECT columns.database_name, columns.schema_name, columns.table_name, list( + struct_pack(column_name, data_type, is_primary_key := c.column_index IS NOT NULL) order by column_index), + t.estimated_size AS estimated_size, t.table_oid AS table_oid +FROM duckdb_columns() columns +LEFT JOIN duckdb_tables() t USING (table_oid) +LEFT JOIN ( + SELECT table_oid, UNNEST(constraint_column_indexes)+1 column_index + FROM duckdb_constraints() + WHERE constraint_type='PRIMARY KEY') c +USING (table_oid, column_index) +WHERE NOT columns.internal AND columns.table_name ILIKE '%%' +GROUP BY ALL; From d64e9942e79c4254df71ab9bb98d5d472bc4f680 Mon Sep 17 00:00:00 2001 From: Mytherin Date: Sat, 14 Mar 2026 21:34:46 +0100 Subject: [PATCH 2/2] Move duckdb to v1.5.0 --- duckdb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/duckdb b/duckdb index c53f190..3a3967a 160000 --- a/duckdb +++ b/duckdb @@ -1 +1 @@ -Subproject commit c53f19074c01a0e8a039785a8814799c163857cb +Subproject commit 3a3967aa8190d0a2d1931d4ca4f5d920760030b4