add getters/setters for CatalogEntry, to better control the parent<->…

…child relationship

src/catalog/catalog_entry.cpp

@@ -41,6 +41,34 @@ string CatalogEntry::ToSQL() const {
 	throw InternalException("Unsupported catalog type for ToSQL()");
 }
 
+void CatalogEntry::SetChild(unique_ptr<CatalogEntry> child_p) {
+	child = std::move(child_p);
+	if (child) {
+		child->parent = this;
+	}
+}
+
+unique_ptr<CatalogEntry> CatalogEntry::TakeChild() {
+	if (child) {
+		child->parent = nullptr;
+	}
+	return std::move(child);
+}
+
+bool CatalogEntry::HasChild() const {
+	return child != nullptr;
+}
+bool CatalogEntry::HasParent() const {
+	return parent != nullptr;
+}
+
+CatalogEntry &CatalogEntry::Child() {
+	return *child;
+}
+optional_ptr<CatalogEntry> CatalogEntry::Parent() {
+	return parent;
+}
+
 Catalog &CatalogEntry::ParentCatalog() {
 	throw InternalException("CatalogEntry::ParentCatalog called on catalog entry without catalog");
 }

src/catalog/catalog_set.cpp

@@ -61,8 +61,7 @@ void CatalogSet::PutEntry(EntryIndex index, unique_ptr<CatalogEntry> catalog_ent
 	if (entry == entries.end()) {
 		throw InternalException("Entry with entry index \"%llu\" does not exist", index.GetIndex());
 	}
-	catalog_entry->child = entry->second.TakeEntry();
-	catalog_entry->child->parent = catalog_entry.get();
+	catalog_entry->SetChild(entry->second.TakeEntry());
 	entry->second.SetEntry(std::move(catalog_entry));
 }
 
@@ -145,7 +144,7 @@ bool CatalogSet::CreateEntry(CatalogTransaction transaction, const string &name,
 	// push the old entry in the undo buffer for this transaction
 	if (transaction.transaction) {
 		auto &dtransaction = transaction.transaction->Cast<DuckTransaction>();
-		dtransaction.PushCatalogEntry(*value_ptr->child);
+		dtransaction.PushCatalogEntry(value_ptr->Child());
 	}
 	return true;
 }
@@ -262,7 +261,7 @@ bool CatalogSet::AlterEntry(CatalogTransaction transaction, const string &name,
 	// push the old entry in the undo buffer for this transaction
 	if (transaction.transaction) {
 		auto &dtransaction = transaction.transaction->Cast<DuckTransaction>();
-		dtransaction.PushCatalogEntry(*new_entry->child, stream.GetData(), stream.GetPosition());
+		dtransaction.PushCatalogEntry(new_entry->Child(), stream.GetData(), stream.GetPosition());
 	}
 
 	// Check the dependency manager to verify that there are no conflicting dependencies with this alter
@@ -309,7 +308,7 @@ void CatalogSet::DropEntryInternal(CatalogTransaction transaction, EntryIndex en
 	// push the old entry in the undo buffer for this transaction
 	if (transaction.transaction) {
 		auto &dtransaction = transaction.transaction->Cast<DuckTransaction>();
-		dtransaction.PushCatalogEntry(*value_ptr->child);
+		dtransaction.PushCatalogEntry(value_ptr->Child());
 	}
 }
 
@@ -341,22 +340,24 @@ DuckCatalog &CatalogSet::GetCatalog() {
 
 void CatalogSet::CleanupEntry(CatalogEntry &catalog_entry) {
 	// destroy the backed up entry: it is no longer required
-	D_ASSERT(catalog_entry.parent);
-	if (catalog_entry.parent->type != CatalogType::UPDATED_ENTRY) {
+	auto parent_p = catalog_entry.Parent();
+	D_ASSERT(parent_p);
+	if (parent_p->type != CatalogType::UPDATED_ENTRY) {
 		lock_guard<mutex> write_lock(catalog.GetWriteLock());
 		lock_guard<mutex> lock(catalog_lock);
 		if (!catalog_entry.deleted) {
 			// delete the entry from the dependency manager, if it is not deleted yet
 			D_ASSERT(catalog_entry.ParentCatalog().IsDuckCatalog());
 			catalog_entry.ParentCatalog().Cast<DuckCatalog>().GetDependencyManager().EraseObject(catalog_entry);
 		}
-		auto parent = catalog_entry.parent;
-		parent->child = std::move(catalog_entry.child);
-		if (parent->deleted && !parent->child && !parent->parent) {
-			auto mapping_entry = mapping.find(parent->name);
+		parent_p = catalog_entry.Parent();
+		auto &parent = *parent_p;
+		parent.SetChild(catalog_entry.TakeChild());
+		if (parent.deleted && !parent.HasChild() && !parent.HasParent()) {
+			auto mapping_entry = mapping.find(parent.name);
 			D_ASSERT(mapping_entry != mapping.end());
 			auto &entry = mapping_entry->second->index.GetEntry();
-			if (&entry == parent.get()) {
+			if (&entry == parent_p.get()) {
 				mapping.erase(mapping_entry);
 			}
 		}
@@ -432,23 +433,23 @@ catalog_entry_t CatalogSet::GenerateCatalogEntryIndex() {
 
 CatalogEntry &CatalogSet::GetEntryForTransaction(CatalogTransaction transaction, CatalogEntry &current) {
 	reference<CatalogEntry> entry(current);
-	while (entry.get().child) {
+	while (entry.get().HasChild()) {
 		if (UseTimestamp(transaction, entry.get().timestamp)) {
 			break;
 		}
-		entry = *entry.get().child;
+		entry = entry.get().Child();
 	}
 	return entry.get();
 }
 
 CatalogEntry &CatalogSet::GetCommittedEntry(CatalogEntry &current) {
 	reference<CatalogEntry> entry(current);
-	while (entry.get().child) {
+	while (entry.get().HasChild()) {
 		if (entry.get().timestamp < TRANSACTION_ID_START) {
 			// this entry is committed: use it
 			break;
 		}
-		entry = *entry.get().child;
+		entry = entry.get().Child();
 	}
 	return entry.get();
 }
@@ -555,7 +556,7 @@ void CatalogSet::Undo(CatalogEntry &entry) {
 	// and entry->parent has to be removed ("rolled back")
 
 	// i.e. we have to place (entry) as (entry->parent) again
-	auto &to_be_removed_node = *entry.parent;
+	auto &to_be_removed_node = *entry.Parent();
 
 	if (!to_be_removed_node.deleted) {
 		// delete the entry from the dependency manager as well
@@ -572,17 +573,15 @@ void CatalogSet::Undo(CatalogEntry &entry) {
 			mapping.erase(removed_entry);
 		}
 	}
-	if (to_be_removed_node.parent) {
+	if (to_be_removed_node.HasParent()) {
 		// if the to be removed node has a parent, set the child pointer to the
 		// to be restored node
-		to_be_removed_node.parent->child = std::move(to_be_removed_node.child);
-		entry.parent = to_be_removed_node.parent;
+		to_be_removed_node.Parent()->SetChild(to_be_removed_node.TakeChild());
 	} else {
 		// otherwise we need to update the base entry tables
 		auto &name = entry.name;
-		to_be_removed_node.child->SetAsRoot();
-		mapping[name]->index.SetEntry(std::move(to_be_removed_node.child));
-		entry.parent = nullptr;
+		to_be_removed_node.Child().SetAsRoot();
+		mapping[name]->index.SetEntry(to_be_removed_node.TakeChild());
 	}
 
 	// restore the name if it was deleted

src/include/duckdb/catalog/catalog_entry.hpp

@@ -49,6 +49,8 @@ class CatalogEntry {
 	bool internal;
 	//! Timestamp at which the catalog entry was created
 	atomic<transaction_t> timestamp;
+
+private:
 	//! Child entry
 	unique_ptr<CatalogEntry> child;
 	//! Parent entry (the node that dependents_map this node)
@@ -77,6 +79,14 @@ class CatalogEntry {
 	void Serialize(Serializer &serializer) const;
 	static unique_ptr<CreateInfo> Deserialize(Deserializer &deserializer);
 
+public:
+	void SetChild(unique_ptr<CatalogEntry> child);
+	unique_ptr<CatalogEntry> TakeChild();
+	bool HasChild() const;
+	bool HasParent() const;
+	CatalogEntry &Child();
+	optional_ptr<CatalogEntry> Parent();
+
 public:
 	template <class TARGET>
 	TARGET &Cast() {

src/transaction/commit_state.cpp

@@ -35,13 +35,13 @@ void CommitState::SwitchTable(DataTableInfo *table_info, UndoFlags new_op) {
 }
 
 void CommitState::WriteCatalogEntry(CatalogEntry &entry, data_ptr_t dataptr) {
-	if (entry.temporary || entry.parent->temporary) {
+	if (entry.temporary || entry.Parent()->temporary) {
 		return;
 	}
 	D_ASSERT(log);
 	// look at the type of the parent entry
-	auto parent = entry.parent;
-	switch (parent->type) {
+	auto &parent = *entry.Parent();
+	switch (parent.type) {
 	case CatalogType::TABLE_ENTRY:
 		if (entry.type == CatalogType::TABLE_ENTRY) {
 			auto &table_entry = entry.Cast<DuckTableEntry>();
@@ -66,15 +66,15 @@ void CommitState::WriteCatalogEntry(CatalogEntry &entry, data_ptr_t dataptr) {
 			log->WriteAlter(alter_info);
 		} else {
 			// CREATE TABLE statement
-			log->WriteCreateTable(parent->Cast<TableCatalogEntry>());
+			log->WriteCreateTable(parent.Cast<TableCatalogEntry>());
 		}
 		break;
 	case CatalogType::SCHEMA_ENTRY:
 		if (entry.type == CatalogType::SCHEMA_ENTRY) {
 			// ALTER TABLE statement, skip it
 			return;
 		}
-		log->WriteCreateSchema(parent->Cast<SchemaCatalogEntry>());
+		log->WriteCreateSchema(parent.Cast<SchemaCatalogEntry>());
 		break;
 	case CatalogType::VIEW_ENTRY:
 		if (entry.type == CatalogType::VIEW_ENTRY) {
@@ -95,23 +95,23 @@ void CommitState::WriteCatalogEntry(CatalogEntry &entry, data_ptr_t dataptr) {
 			auto &alter_info = parse_info->Cast<AlterInfo>();
 			log->WriteAlter(alter_info);
 		} else {
-			log->WriteCreateView(parent->Cast<ViewCatalogEntry>());
+			log->WriteCreateView(parent.Cast<ViewCatalogEntry>());
 		}
 		break;
 	case CatalogType::SEQUENCE_ENTRY:
-		log->WriteCreateSequence(parent->Cast<SequenceCatalogEntry>());
+		log->WriteCreateSequence(parent.Cast<SequenceCatalogEntry>());
 		break;
 	case CatalogType::MACRO_ENTRY:
-		log->WriteCreateMacro(parent->Cast<ScalarMacroCatalogEntry>());
+		log->WriteCreateMacro(parent.Cast<ScalarMacroCatalogEntry>());
 		break;
 	case CatalogType::TABLE_MACRO_ENTRY:
-		log->WriteCreateTableMacro(parent->Cast<TableMacroCatalogEntry>());
+		log->WriteCreateTableMacro(parent.Cast<TableMacroCatalogEntry>());
 		break;
 	case CatalogType::INDEX_ENTRY:
-		log->WriteCreateIndex(parent->Cast<IndexCatalogEntry>());
+		log->WriteCreateIndex(parent.Cast<IndexCatalogEntry>());
 		break;
 	case CatalogType::TYPE_ENTRY:
-		log->WriteCreateType(parent->Cast<TypeCatalogEntry>());
+		log->WriteCreateType(parent.Cast<TypeCatalogEntry>());
 		break;
 	case CatalogType::DELETED_ENTRY:
 		switch (entry.type) {
@@ -246,16 +246,16 @@ void CommitState::CommitEntry(UndoFlags type, data_ptr_t data) {
 	case UndoFlags::CATALOG_ENTRY: {
 		// set the commit timestamp of the catalog entry to the given id
 		auto catalog_entry = Load<CatalogEntry *>(data);
-		D_ASSERT(catalog_entry->parent);
+		D_ASSERT(catalog_entry->HasParent());
 
 		auto &catalog = catalog_entry->ParentCatalog();
 		D_ASSERT(catalog.IsDuckCatalog());
 
 		// Grab a write lock on the catalog
 		auto &duck_catalog = catalog.Cast<DuckCatalog>();
 		lock_guard<mutex> write_lock(duck_catalog.GetWriteLock());
-		catalog_entry->set->UpdateTimestamp(*catalog_entry->parent, commit_id);
-		if (catalog_entry->name != catalog_entry->parent->name) {
+		catalog_entry->set->UpdateTimestamp(*catalog_entry->Parent(), commit_id);
+		if (catalog_entry->name != catalog_entry->Parent()->name) {
 			catalog_entry->set->UpdateTimestamp(*catalog_entry, commit_id);
 		}
 		if (HAS_LOG) {
@@ -304,9 +304,9 @@ void CommitState::RevertCommit(UndoFlags type, data_ptr_t data) {
 	case UndoFlags::CATALOG_ENTRY: {
 		// set the commit timestamp of the catalog entry to the given id
 		auto catalog_entry = Load<CatalogEntry *>(data);
-		D_ASSERT(catalog_entry->parent);
-		catalog_entry->set->UpdateTimestamp(*catalog_entry->parent, transaction_id);
-		if (catalog_entry->name != catalog_entry->parent->name) {
+		D_ASSERT(catalog_entry->HasParent());
+		catalog_entry->set->UpdateTimestamp(*catalog_entry->Parent(), transaction_id);
+		if (catalog_entry->name != catalog_entry->Parent()->name) {
 			catalog_entry->set->UpdateTimestamp(*catalog_entry, transaction_id);
 		}
 		break;