identified issue caused by the EntryDropper, I think this is not even…

… necessary
Tishj · Oct 28, 2023 · 28560fb · 28560fb
1 parent 7e0e91c
commit 28560fb
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 24 deletions.
diff --git a/src/catalog/catalog_set.cpp b/src/catalog/catalog_set.cpp
@@ -280,24 +280,6 @@ bool CatalogSet::AlterEntry(CatalogTransaction transaction, const string &name,
 	return true;
 }
 
-void CatalogSet::DropEntryDependencies(CatalogTransaction transaction, EntryIndex &entry_index, CatalogEntry &entry,
-                                       bool cascade) {
-	// Stores the deleted value of the entry before starting the process
-	EntryDropper dropper(entry_index);
-
-	// To correctly delete the object and its dependencies, it temporarily is set to deleted.
-	entry_index.GetEntry().deleted = true;
-
-	// 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);
-
-	// dropper destructor is called here
-	// the destructor makes sure to return the value to the previous state
-	// dropper.~EntryDropper()
-}
-
 void CatalogSet::DropEntryInternal(CatalogTransaction transaction, EntryIndex entry_index, CatalogEntry &entry,
                                    bool cascade) {
 
@@ -330,9 +312,20 @@ bool CatalogSet::DropEntry(CatalogTransaction transaction, const string &name, b
 	if (entry->internal && !allow_drop_internal) {
 		throw CatalogException("Cannot drop entry \"%s\" because it is an internal system entry", entry->name);
 	}
-	write_lock.unlock();
-	DropEntryDependencies(transaction, entry_index, *entry, cascade);
-	write_lock.lock();
+	{
+		// Stores the deleted value of the entry before starting the process
+		EntryDropper dropper(entry_index);
+
+		// To correctly delete the object and its dependencies, it temporarily is set to deleted.
+		entry_index.GetEntry().deleted = true;
+
+		// 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);
 	DropEntryInternal(transaction, std::move(entry_index), *entry, cascade);

diff --git a/src/include/duckdb/catalog/catalog_set.hpp b/src/include/duckdb/catalog/catalog_set.hpp
@@ -163,8 +163,6 @@ class CatalogSet {
 	optional_ptr<MappingValue> GetLatestMapping(const string &name);
 	void PutMapping(CatalogTransaction transaction, const string &name, EntryIndex entry_index);
 	void DeleteMapping(CatalogTransaction transaction, const string &name);
-	void DropEntryDependencies(CatalogTransaction transaction, EntryIndex &entry_index, CatalogEntry &entry,
-	                           bool cascade);
 
 	//! Create all default entries
 	void CreateDefaultEntries(CatalogTransaction transaction, unique_lock<mutex> &lock);

diff --git a/test/fuzzer/pedro/concurrent_catalog_usage.test b/test/fuzzer/pedro/concurrent_catalog_usage.test
@@ -5,7 +5,7 @@
 statement ok
 CREATE TABLE t2 AS (SELECT 42);
 
-concurrentloop i 1 10
+concurrentloop i 1 100
 
 statement maybe
 CREATE OR REPLACE TABLE t2 AS (SELECT -54124033386577348004002656426531535114 FROM t2 LIMIT 70%);