cleaning up, identified issue, have a fix in mind

src/catalog/catalog_entry/dependency_set_catalog_entry.cpp

@@ -5,12 +5,14 @@
 #include "duckdb/common/printer.hpp"
 #include "duckdb/catalog/mapping_value.hpp"
 #include "duckdb/catalog/dependency_manager.hpp"
+#include "duckdb/catalog/catalog.hpp"
 
 namespace duckdb {
 
-DependencySetCatalogEntry::DependencySetCatalogEntry(Catalog &catalog, const string &name)
+DependencySetCatalogEntry::DependencySetCatalogEntry(Catalog &catalog, DependencyManager &dependency_manager,
+                                                     const string &name)
     : InCatalogEntry(CatalogType::DEPENDENCY_SET, catalog, name), name(name), dependencies(catalog),
-      dependents(catalog) {
+      dependents(catalog), dependency_manager(dependency_manager) {
 }
 
 CatalogSet &DependencySetCatalogEntry::Dependencies() {
@@ -21,6 +23,38 @@ CatalogSet &DependencySetCatalogEntry::Dependents() {
 	return dependents;
 }
 
+void DependencySetCatalogEntry::ScanDependents(
+    CatalogTransaction transaction,
+    const std::function<void(DependencyCatalogEntry &, DependencySetCatalogEntry &)> &callback) {
+	dependents.Scan(transaction, [&](CatalogEntry &other) {
+		D_ASSERT(other.type == CatalogType::DEPENDENCY_ENTRY);
+		auto &other_entry = other.Cast<DependencyCatalogEntry>();
+		auto other_connections_p = dependency_manager.GetDependencySet(transaction, other);
+		if (!other_connections_p) {
+			// Already deleted
+			return;
+		}
+		auto &other_connections = *other_connections_p;
+		callback(other_entry, other_connections);
+	});
+}
+
+void DependencySetCatalogEntry::ScanDependencies(
+    CatalogTransaction transaction,
+    const std::function<void(DependencyCatalogEntry &, DependencySetCatalogEntry &)> &callback) {
+	dependencies.Scan(transaction, [&](CatalogEntry &other) {
+		D_ASSERT(other.type == CatalogType::DEPENDENCY_ENTRY);
+		auto &other_entry = other.Cast<DependencyCatalogEntry>();
+		auto other_connections_p = dependency_manager.GetDependencySet(transaction, other);
+		if (!other_connections_p) {
+			// Already deleted
+			return;
+		}
+		auto &other_connections = *other_connections_p;
+		callback(other_entry, other_connections);
+	});
+}
+
 DependencySetCatalogEntry::~DependencySetCatalogEntry() {
 }
 
@@ -63,6 +97,9 @@ void DependencySetCatalogEntry::AddDependency(CatalogTransaction transaction, Ca
 	auto dependency = make_uniq<DependencyCatalogEntry>(DependencyConnectionType::DEPENDENCY, catalog, to_add, type);
 	auto dependency_name = dependency->name;
 	D_ASSERT(dependency_name != name);
+	if (catalog.IsTemporaryCatalog()) {
+		dependency->temporary = true;
+	}
 	dependencies.CreateEntry(transaction, dependency_name, std::move(dependency), empty_dependencies);
 }
 void DependencySetCatalogEntry::AddDependent(CatalogTransaction transaction, CatalogEntry &to_add,
@@ -71,6 +108,9 @@ void DependencySetCatalogEntry::AddDependent(CatalogTransaction transaction, Cat
 	auto dependent = make_uniq<DependencyCatalogEntry>(DependencyConnectionType::DEPENDENT, catalog, to_add, type);
 	auto dependent_name = dependent->name;
 	D_ASSERT(dependent_name != name);
+	if (catalog.IsTemporaryCatalog()) {
+		dependent->temporary = true;
+	}
 	dependents.CreateEntry(transaction, dependent_name, std::move(dependent), empty_dependencies);
 }
 

src/catalog/catalog_set.cpp

@@ -255,24 +255,37 @@ bool CatalogSet::AlterEntry(CatalogTransaction transaction, const string &name,
 	return true;
 }
 
+bool CatalogSet::DropDependencies(CatalogTransaction transaction, const string &name, bool cascade,
+                                  bool allow_drop_internal) {
+	auto entry = GetEntry(transaction, name);
+	if (!entry) {
+		return false;
+	}
+	if (entry->internal && !allow_drop_internal) {
+		throw CatalogException("Cannot drop entry \"%s\" because it is an internal system entry", entry->name);
+	}
+	// check any dependencies of this object
+	D_ASSERT(entry->ParentCatalog().IsDuckCatalog());
+	auto &duck_catalog = entry->ParentCatalog().Cast<DuckCatalog>();
+	duck_catalog.GetDependencyManager().DropObject(transaction, *entry, cascade);
+	return true;
+}
+
 bool CatalogSet::DropEntry(CatalogTransaction transaction, const string &name, bool cascade, bool allow_drop_internal) {
+	if (!DropDependencies(transaction, name, cascade, allow_drop_internal)) {
+		return false;
+	}
 	EntryIndex entry_index;
 	// lock the catalog for writing
 	// we can only delete an entry that exists
-	unique_lock<mutex> write_lock(catalog.GetWriteLock());
+	lock_guard<mutex> write_lock(catalog.GetWriteLock());
 	auto entry = GetEntryInternal(transaction, name, &entry_index);
 	if (!entry) {
 		return false;
 	}
 	if (entry->internal && !allow_drop_internal) {
 		throw CatalogException("Cannot drop entry \"%s\" because it is an internal system entry", entry->name);
 	}
-	// check any dependencies of this object
-	D_ASSERT(entry->ParentCatalog().IsDuckCatalog());
-	auto &duck_catalog = entry->ParentCatalog().Cast<DuckCatalog>();
-	write_lock.unlock();
-	duck_catalog.GetDependencyManager().DropObject(transaction, *entry, cascade);
-	write_lock.lock();
 
 	lock_guard<mutex> read_lock(catalog_lock);
 	// create a new entry and replace the currently stored one
@@ -485,26 +498,18 @@ optional_ptr<CatalogEntry> CatalogSet::CreateDefaultEntry(CatalogTransaction tra
 	return GetEntry(transaction, name);
 }
 
-static bool IncludeEntry(bool include_deleted, bool deleted) {
-	if (!deleted) {
-		return true;
-	}
-	return include_deleted;
-}
-
-optional_ptr<CatalogEntry> CatalogSet::GetEntry(CatalogTransaction transaction, const string &name,
-                                                bool include_deleted) {
+optional_ptr<CatalogEntry> CatalogSet::GetEntry(CatalogTransaction transaction, const string &name) {
 	unique_lock<mutex> lock(catalog_lock);
 	auto mapping_value = GetMapping(transaction, name);
-	if (!mapping_value || !IncludeEntry(include_deleted, mapping_value->deleted)) {
+	if (!mapping_value || mapping_value->deleted) {
 		return CreateDefaultEntry(transaction, name, lock);
 	}
 	// we found an entry for this name
 	// check the version numbers
 
 	auto &catalog_entry = mapping_value->index.GetEntry();
 	auto &current = GetEntryForTransaction(transaction, catalog_entry);
-	const bool entry_is_invalid = !IncludeEntry(include_deleted, current.deleted);
+	const bool entry_is_invalid = current.deleted;
 	const bool entry_is_out_of_date = current.name != name && !UseTimestamp(transaction, mapping_value->timestamp);
 	if (entry_is_invalid || entry_is_out_of_date) {
 		return nullptr;

src/catalog/dependency_manager.cpp

@@ -67,7 +67,7 @@ DependencySetCatalogEntry &DependencyManager::GetOrCreateDependencySet(CatalogTr
 	auto name = MangleName(object);
 	auto connection_p = connections.GetEntry(transaction, name);
 	if (!connection_p) {
-		auto new_connection = make_uniq<DependencySetCatalogEntry>(catalog, name);
+		auto new_connection = make_uniq<DependencySetCatalogEntry>(catalog, *this, name);
 		if (catalog.IsTemporaryCatalog()) {
 			new_connection->temporary = true;
 		}
@@ -190,9 +190,8 @@ DependencyManager::LookupResult DependencyManager::LookupEntry(CatalogTransactio
 	CatalogType type;
 	GetLookupProperties(dependency, schema, name, type);
 
-	// Lookup the schema, 'include_deleted' is true so we can still find the entries that are part of the schema
-	// that is currently being dropped
-	auto schema_entry = catalog.schemas->GetEntry(transaction, schema, /* include_deleted = */ true);
+	// Lookup the schema
+	auto schema_entry = catalog.schemas->GetEntry(transaction, schema);
 	if (type == CatalogType::SCHEMA_ENTRY || !schema_entry) {
 		// This is a schema entry, perform the callback only providing the schema
 		return LookupResult(schema_entry);
@@ -280,13 +279,13 @@ void DependencyManager::DropObject(CatalogTransaction transaction, CatalogEntry
 		to_drop.emplace_back(std::move(lookup));
 	});
 
+	CleanupDependencies(transaction, object);
+
 	for (auto &lookup : to_drop) {
 		auto set = lookup.set;
 		auto entry = lookup.entry;
 		set->DropEntry(transaction, entry->name, cascade);
 	}
-
-	CleanupDependencies(transaction, object);
 }
 
 void DependencyManager::AlterObject(CatalogTransaction transaction, CatalogEntry &old_obj, CatalogEntry &new_obj) {
@@ -420,9 +419,6 @@ void DependencyManager::AddOwnership(CatalogTransaction transaction, CatalogEntr
 	D_ASSERT(!IsSystemEntry(entry));
 	D_ASSERT(!IsSystemEntry(owner));
 
-	// lock the catalog for writing
-	lock_guard<mutex> write_lock(catalog.GetWriteLock());
-
 	// If the owner is already owned by something else, throw an error
 	auto &owner_connections = GetOrCreateDependencySet(transaction, owner);
 	auto &owner_dependents = owner_connections.Dependents();

src/include/duckdb/catalog/catalog_entry/dependency_set_catalog_entry.hpp

@@ -20,15 +20,24 @@
 
 namespace duckdb {
 
+class DependencyManager;
+class DependencyCatalogEntry;
+
 class DependencySetCatalogEntry : public InCatalogEntry {
 public:
-	DependencySetCatalogEntry(Catalog &catalog, const string &name);
+	DependencySetCatalogEntry(Catalog &catalog, DependencyManager &dependency_manager, const string &name);
 	~DependencySetCatalogEntry() override;
 
 public:
 	CatalogSet &Dependencies();
 	CatalogSet &Dependents();
 
+public:
+	void ScanDependents(CatalogTransaction transaction,
+	                    const std::function<void(DependencyCatalogEntry &, DependencySetCatalogEntry &)> &callback);
+	void ScanDependencies(CatalogTransaction transaction,
+	                      const std::function<void(DependencyCatalogEntry &, DependencySetCatalogEntry &)> &callback);
+
 public:
 	// Add Dependencies
 	void AddDependency(CatalogTransaction transaction, CatalogEntry &dependent,
@@ -67,6 +76,7 @@ class DependencySetCatalogEntry : public InCatalogEntry {
 	string name;
 	CatalogSet dependencies;
 	CatalogSet dependents;
+	DependencyManager &dependency_manager;
 };
 
 } // namespace duckdb

src/include/duckdb/catalog/catalog_set.hpp

@@ -113,8 +113,7 @@ class CatalogSet {
 	void CleanupEntry(CatalogEntry &catalog_entry);
 
 	//! Returns the entry with the specified name
-	DUCKDB_API optional_ptr<CatalogEntry> GetEntry(CatalogTransaction transaction, const string &name,
-	                                               bool include_deleted = false);
+	DUCKDB_API optional_ptr<CatalogEntry> GetEntry(CatalogTransaction transaction, const string &name);
 	DUCKDB_API optional_ptr<CatalogEntry> GetEntry(ClientContext &context, const string &name);
 
 	//! Gets the entry that is most similar to the given name (i.e. smallest levenshtein distance), or empty string if
@@ -146,6 +145,8 @@ class CatalogSet {
 	void Verify(Catalog &catalog);
 
 private:
+	bool DropDependencies(CatalogTransaction transaction, const string &name, bool cascade,
+	                      bool allow_drop_internal = false);
 	catalog_entry_t GenerateCatalogEntryIndex();
 	//! Given a root entry, gets the entry valid for this transaction
 	CatalogEntry &GetEntryForTransaction(CatalogTransaction transaction, CatalogEntry &current);

src/include/duckdb/catalog/dependency_manager.hpp

@@ -13,7 +13,6 @@
 #include "duckdb/catalog/dependency.hpp"
 #include "duckdb/catalog/catalog_entry_map.hpp"
 #include "duckdb/catalog/catalog_transaction.hpp"
-#include "duckdb/catalog/catalog_entry/dependency_set_catalog_entry.hpp"
 
 #include <functional>
 
@@ -22,10 +21,12 @@ class DuckCatalog;
 class ClientContext;
 class DependencyList;
 class DependencyCatalogEntry;
+class DependencySetCatalogEntry;
 
 //! The DependencyManager is in charge of managing dependencies between catalog entries
 class DependencyManager {
 	friend class CatalogSet;
+	friend class DependencySetCatalogEntry;
 
 public:
 	explicit DependencyManager(DuckCatalog &catalog);