From c02a09602e7a368b73a2b91d91d57471aa111f8a Mon Sep 17 00:00:00 2001 From: DuckDB Labs GitHub Bot Date: Mon, 22 Dec 2025 05:04:35 +0000 Subject: [PATCH] Update vendored DuckDB sources to c46a01b579 --- src/duckdb/src/common/error_data.cpp | 3 +- src/duckdb/src/function/table/read_duckdb.cpp | 3 +- .../function/table/version/pragma_version.cpp | 6 +- .../include/duckdb/main/attached_database.hpp | 21 ++++-- .../include/duckdb/main/database_manager.hpp | 6 +- .../duckdb/transaction/meta_transaction.hpp | 21 +++--- src/duckdb/src/main/attached_database.cpp | 66 ++++++++++++------- src/duckdb/src/main/database.cpp | 2 +- src/duckdb/src/main/database_manager.cpp | 11 ++-- .../src/transaction/meta_transaction.cpp | 9 +++ .../src/transaction/transaction_context.cpp | 10 +-- 11 files changed, 99 insertions(+), 59 deletions(-) diff --git a/src/duckdb/src/common/error_data.cpp b/src/duckdb/src/common/error_data.cpp index 44f70a085..8b8da300d 100644 --- a/src/duckdb/src/common/error_data.cpp +++ b/src/duckdb/src/common/error_data.cpp @@ -80,9 +80,8 @@ void ErrorData::Throw(const string &prepended_message) const { if (!prepended_message.empty()) { string new_message = prepended_message + raw_message; throw Exception(extra_info, type, new_message); - } else { - throw Exception(extra_info, type, raw_message); } + throw Exception(extra_info, type, raw_message); } const ExceptionType &ErrorData::Type() const { diff --git a/src/duckdb/src/function/table/read_duckdb.cpp b/src/duckdb/src/function/table/read_duckdb.cpp index 4f8cfac66..9f870547b 100644 --- a/src/duckdb/src/function/table/read_duckdb.cpp +++ b/src/duckdb/src/function/table/read_duckdb.cpp @@ -189,8 +189,9 @@ AttachedDatabaseWrapper::AttachedDatabaseWrapper(ClientContext &context, AttachedDatabaseWrapper::~AttachedDatabaseWrapper() { if (attached_database) { auto &db_manager = DatabaseManager::Get(context); - db_manager.DetachDatabase(context, attached_database->GetName(), OnEntryNotFound::RETURN_NULL); + auto name = attached_database->GetName(); attached_database.reset(); + db_manager.DetachDatabase(context, name, OnEntryNotFound::RETURN_NULL); } } DuckDBReader::DuckDBReader(ClientContext &context_p, OpenFileInfo file_p, const DuckDBFileReaderOptions &options) diff --git a/src/duckdb/src/function/table/version/pragma_version.cpp b/src/duckdb/src/function/table/version/pragma_version.cpp index a26ae395b..12500c8f1 100644 --- a/src/duckdb/src/function/table/version/pragma_version.cpp +++ b/src/duckdb/src/function/table/version/pragma_version.cpp @@ -1,5 +1,5 @@ #ifndef DUCKDB_PATCH_VERSION -#define DUCKDB_PATCH_VERSION "0-dev4877" +#define DUCKDB_PATCH_VERSION "0-dev4892" #endif #ifndef DUCKDB_MINOR_VERSION #define DUCKDB_MINOR_VERSION 5 @@ -8,10 +8,10 @@ #define DUCKDB_MAJOR_VERSION 1 #endif #ifndef DUCKDB_VERSION -#define DUCKDB_VERSION "v1.5.0-dev4877" +#define DUCKDB_VERSION "v1.5.0-dev4892" #endif #ifndef DUCKDB_SOURCE_ID -#define DUCKDB_SOURCE_ID "be5250ba48" +#define DUCKDB_SOURCE_ID "c46a01b579" #endif #include "duckdb/function/table/system_functions.hpp" #include "duckdb/main/database.hpp" diff --git a/src/duckdb/src/include/duckdb/main/attached_database.hpp b/src/duckdb/src/include/duckdb/main/attached_database.hpp index b013bee7e..7a0d95a1b 100644 --- a/src/duckdb/src/include/duckdb/main/attached_database.hpp +++ b/src/duckdb/src/include/duckdb/main/attached_database.hpp @@ -8,9 +8,6 @@ #pragma once -#include "duckdb/common/common.hpp" -#include "duckdb/common/case_insensitive_map.hpp" -#include "duckdb/common/mutex.hpp" #include "duckdb/main/config.hpp" #include "duckdb/catalog/catalog_entry.hpp" @@ -39,6 +36,10 @@ enum class AttachVisibility { SHOWN, HIDDEN }; //! Use this mode with caution, as it disables recovery from crashes for the file. enum class RecoveryMode : uint8_t { DEFAULT = 0, NO_WAL_WRITES = 1 }; +//! CHECKPOINT: Throws, if the checkpoint fails. Always cleans up. +//! TRY_CHECKPOINT: Does not throw when failing a checkpoint. Always cleans up. +enum class DatabaseCloseAction { CHECKPOINT, TRY_CHECKPOINT }; + class DatabaseFilePathManager; struct StoredDatabasePath { @@ -49,7 +50,6 @@ struct StoredDatabasePath { DatabaseFilePathManager &manager; string path; -public: void OnDetach(); }; @@ -71,7 +71,7 @@ struct AttachOptions { unordered_map options; //! (optionally) a catalog can be provided with a default table QualifiedName default_table; - //! Whether or not this is the main database + //! Whether this is the main database. bool is_main_database = false; //! The visibility of the attached database AttachVisibility visibility = AttachVisibility::SHOWN; @@ -94,7 +94,8 @@ class AttachedDatabase : public CatalogEntry, public enable_shared_from_this context = nullptr); void FinalizeLoad(optional_ptr context); - void Close(); + //! Close the database before shutting it down. + void Close(const DatabaseCloseAction action); Catalog &ParentCatalog() override; const Catalog &ParentCatalog() const override; @@ -136,6 +137,9 @@ class AttachedDatabase : public CatalogEntry, public enable_shared_from_this &attached_database); private: DatabaseInstance &db; @@ -150,7 +154,12 @@ class AttachedDatabase : public CatalogEntry, public enable_shared_from_this attach_options; + +private: + //! Clean any (shared) resources held by the database. + void Cleanup(); }; } // namespace duckdb diff --git a/src/duckdb/src/include/duckdb/main/database_manager.hpp b/src/duckdb/src/include/duckdb/main/database_manager.hpp index b516e858f..a2f64e6a5 100644 --- a/src/duckdb/src/include/duckdb/main/database_manager.hpp +++ b/src/duckdb/src/include/duckdb/main/database_manager.hpp @@ -8,12 +8,8 @@ #pragma once -#include "duckdb/common/atomic.hpp" #include "duckdb/common/case_insensitive_map.hpp" #include "duckdb/common/common.hpp" -#include "duckdb/common/enums/access_mode.hpp" -#include "duckdb/common/enums/on_entry_not_found.hpp" -#include "duckdb/common/mutex.hpp" #include "duckdb/common/optional_ptr.hpp" #include "duckdb/main/config.hpp" #include "duckdb/parser/parsed_data/attach_info.hpp" @@ -82,7 +78,7 @@ class DatabaseManager { idx_t ApproxDatabaseCount(); //! Removes all databases from the catalog set. This is necessary for the database instance's destructor, //! as the database manager has to be alive when destroying the catalog set objects. - void ResetDatabases(unique_ptr &scheduler); + void ResetDatabases(); transaction_t GetNewQueryNumber() { return current_query_number++; diff --git a/src/duckdb/src/include/duckdb/transaction/meta_transaction.hpp b/src/duckdb/src/include/duckdb/transaction/meta_transaction.hpp index 8209963a4..99416795e 100644 --- a/src/duckdb/src/include/duckdb/transaction/meta_transaction.hpp +++ b/src/duckdb/src/include/duckdb/transaction/meta_transaction.hpp @@ -9,14 +9,13 @@ #pragma once #include "duckdb/common/common.hpp" -#include "duckdb/common/atomic.hpp" #include "duckdb/main/valid_checker.hpp" #include "duckdb/common/types/timestamp.hpp" -#include "duckdb/common/unordered_map.hpp" #include "duckdb/common/optional_ptr.hpp" #include "duckdb/common/reference_map.hpp" #include "duckdb/common/error_data.hpp" #include "duckdb/common/case_insensitive_map.hpp" +#include "duckdb/main/attached_database.hpp" namespace duckdb { class AttachedDatabase; @@ -63,6 +62,8 @@ class MetaTransaction { ErrorData Commit(); void Rollback(); + // Finalize the transaction after a COMMIT of ROLLBACK. + void Finalize(); idx_t GetActiveQuery(); void SetActiveQuery(transaction_t query_number); @@ -82,21 +83,21 @@ class MetaTransaction { void DetachDatabase(AttachedDatabase &database); private: - //! Lock to prevent all_transactions and transactions from getting out of sync + //! Lock to prevent all_transactions and transactions from getting out of sync. mutex lock; - //! The set of active transactions for each database + //! The set of active transactions for each database. reference_map_t transactions; - //! The set of transactions in order of when they were started + //! The set of referenced databases in invocation order. vector> all_transactions; - //! The database we are modifying - we can only modify one database per transaction + //! The database we are modifying. We can only modify one database per meta transaction. optional_ptr modified_database; - //! Whether or not the meta transaction is marked as read only + //! Whether the meta transaction is marked as read only. bool is_read_only; - //! Lock for referenced_databases + //! Lock for referenced_databases. mutex referenced_database_lock; - //! The set of used / referenced databases + //! The set of used (referenced) databases. reference_map_t> referenced_databases; - //! Map of name -> used database for databases that are in-use by this transaction + //! Map of name -> database for databases that are in-use by this transaction. case_insensitive_map_t> used_databases; }; diff --git a/src/duckdb/src/main/attached_database.cpp b/src/duckdb/src/main/attached_database.cpp index fa7c2c940..3a2ce8896 100644 --- a/src/duckdb/src/main/attached_database.cpp +++ b/src/duckdb/src/main/attached_database.cpp @@ -152,7 +152,10 @@ AttachedDatabase::AttachedDatabase(DatabaseInstance &db, Catalog &catalog_p, Sto } AttachedDatabase::~AttachedDatabase() { - Close(); + // FIXME: Theoretically, we should catch all places higher up the call stack where the + // FIXME: shared pointer goes out of scope and we need to invoke a checkpoint. + // FIXME: However, for now, we keep this to avoid unintentional behavior / oversights. + Close(DatabaseCloseAction::TRY_CHECKPOINT); } bool AttachedDatabase::IsSystem() const { @@ -195,6 +198,16 @@ string AttachedDatabase::ExtractDatabaseName(const string &dbpath, FileSystem &f return name; } +void AttachedDatabase::InvokeCloseIfLastReference(shared_ptr &attached_db) { + { + lock_guard guard(attached_db->close_lock); + if (attached_db.use_count() == 1) { + attached_db->Close(DatabaseCloseAction::CHECKPOINT); + } + } + attached_db.reset(); +} + void AttachedDatabase::Initialize(optional_ptr context) { if (IsSystem()) { catalog->Initialize(context, true); @@ -258,40 +271,47 @@ void AttachedDatabase::OnDetach(ClientContext &context) { } } -void AttachedDatabase::Close() { +void AttachedDatabase::Close(const DatabaseCloseAction action) { if (is_closed) { return; } D_ASSERT(catalog); is_closed = true; - // shutting down: attempt to checkpoint the database - // but only if we are not cleaning up as part of an exception unwind - if (!Exception::UncaughtException() && storage && !ValidChecker::IsInvalidated(db)) { - if (!storage->InMemory()) { + try { + auto create_checkpoint = true; + if (action == DatabaseCloseAction::TRY_CHECKPOINT && Exception::UncaughtException()) { + create_checkpoint = false; + } else if (!storage || storage->InMemory() || ValidChecker::IsInvalidated(db)) { + create_checkpoint = false; + } + + if (create_checkpoint) { + auto &config = DBConfig::GetConfig(db); + if (config.options.checkpoint_on_shutdown) { + CheckpointOptions options; + options.wal_action = CheckpointWALAction::DELETE_WAL; + storage->CreateCheckpoint(QueryContext(), options); + } + } + } catch (std::exception &ex) { + ErrorData data(ex); + if (action == DatabaseCloseAction::TRY_CHECKPOINT) { try { - auto &config = DBConfig::GetConfig(db); - if (config.options.checkpoint_on_shutdown) { - CheckpointOptions options; - options.wal_action = CheckpointWALAction::DELETE_WAL; - storage->CreateCheckpoint(QueryContext(), options); - } - } catch (std::exception &ex) { - ErrorData data(ex); - try { - DUCKDB_LOG_ERROR(db, "AttachedDatabase::Close()\t\t" + data.Message()); - } catch (...) { // NOLINT - } + DUCKDB_LOG_ERROR(db, "Silent exception in AttachedDatabase::Close():\t" + data.Message()); } catch (...) { // NOLINT } + } else { + Cleanup(); + data.Throw("Detached database '" + name + "', but CHECKPOINT during DETACH failed. \n"); } - try { - // destroy the storage - storage->Destroy(); - } catch (...) { // NOLINT - } + } catch (...) { // NOLINT } + Cleanup(); +} + +void AttachedDatabase::Cleanup() { transaction_manager.reset(); catalog.reset(); storage.reset(); diff --git a/src/duckdb/src/main/database.cpp b/src/duckdb/src/main/database.cpp index 6e68d4e8d..6badf7f9d 100644 --- a/src/duckdb/src/main/database.cpp +++ b/src/duckdb/src/main/database.cpp @@ -77,7 +77,7 @@ DatabaseInstance::DatabaseInstance() : db_validity(*this) { DatabaseInstance::~DatabaseInstance() { // destroy all attached databases if (db_manager) { - db_manager->ResetDatabases(scheduler); + db_manager->ResetDatabases(); } // destroy child elements connection_manager.reset(); diff --git a/src/duckdb/src/main/database_manager.cpp b/src/duckdb/src/main/database_manager.cpp index e4f3f89c5..80b78ee57 100644 --- a/src/duckdb/src/main/database_manager.cpp +++ b/src/duckdb/src/main/database_manager.cpp @@ -208,6 +208,9 @@ void DatabaseManager::DetachDatabase(ClientContext &context, const string &name, } attached_db->OnDetach(context); + + // DetachInternal removes the AttachedDatabase from the list of databases that can be referenced. + AttachedDatabase::InvokeCloseIfLastReference(attached_db); } void DatabaseManager::Alter(ClientContext &context, AlterInfo &info) { @@ -394,10 +397,10 @@ vector> DatabaseManager::GetDatabases() { return result; } -void DatabaseManager::ResetDatabases(unique_ptr &scheduler) { - auto databases = GetDatabases(); - for (auto &entry : databases) { - entry->Close(); +void DatabaseManager::ResetDatabases() { + auto shared_db_pointers = GetDatabases(); + for (auto &entry : shared_db_pointers) { + entry->Close(DatabaseCloseAction::TRY_CHECKPOINT); entry.reset(); } } diff --git a/src/duckdb/src/transaction/meta_transaction.cpp b/src/duckdb/src/transaction/meta_transaction.cpp index 02ebe8704..7e7548727 100644 --- a/src/duckdb/src/transaction/meta_transaction.cpp +++ b/src/duckdb/src/transaction/meta_transaction.cpp @@ -174,6 +174,15 @@ void MetaTransaction::Rollback() { } } +void MetaTransaction::Finalize() { + // Try to checkpoint any attached databases potentially still held by this transaction. + for (auto &database : referenced_databases) { + // If the use count is down to one, then we already detached the database. + // That means new transactions can no longer obtain a shared pointer to it. + AttachedDatabase::InvokeCloseIfLastReference(database.second); + } +} + idx_t MetaTransaction::GetActiveQuery() { return active_query; } diff --git a/src/duckdb/src/transaction/transaction_context.cpp b/src/duckdb/src/transaction/transaction_context.cpp index deaae2029..a88f831b0 100644 --- a/src/duckdb/src/transaction/transaction_context.cpp +++ b/src/duckdb/src/transaction/transaction_context.cpp @@ -6,6 +6,7 @@ #include "duckdb/main/client_data.hpp" #include "duckdb/main/database.hpp" #include "duckdb/transaction/meta_transaction.hpp" +#include "duckdb/main/attached_database.hpp" namespace duckdb { @@ -55,11 +56,11 @@ void TransactionContext::Commit() { s->TransactionRollback(*transaction, context, error); } throw TransactionException("Failed to commit: %s", error.RawMessage()); - } else { - for (auto &state : context.registered_state->States()) { - state->TransactionCommit(*transaction, context); - } } + for (auto &state : context.registered_state->States()) { + state->TransactionCommit(*transaction, context); + } + transaction->Finalize(); } void TransactionContext::SetAutoCommit(bool value) { @@ -94,6 +95,7 @@ void TransactionContext::Rollback(optional_ptr error) { if (rollback_error.HasError()) { rollback_error.Throw(); } + transaction->Finalize(); } void TransactionContext::ClearTransaction() {