ronantakizawa/codereview-bench huggingface dataset:
Repository: duckdb/duckdb
File: src/execution/index/art/iterator.cpp (C++)

Code before:

		// Go to the leftmost entry in the current node.
		uint8_t byte = 0;
		auto next = current_node.GetNextChild(art, byte);
		D_ASSERT(next);

		// Move to the leftmost node.
		current_key.Push(byte);
		if (status == GateStatus::GATE_SET) {
			row_id[nested_depth] = byte;
			nested_depth++;
			D_ASSERT(nested_depth < Prefix::ROW_ID_SIZE);
		}
		nodes.emplace(current_node, byte);
		current_node = *next;
	}
}

bool Iterator::LowerBound(const Node &node, const ARTKey &key, const bool equal) {
	auto current_node = node;
	idx_t depth = 0;

	while (true) {
		if (!current_node.HasMetadata()) {
			return false;
		}

		// We found any leaf node, or a gate.
		if (current_node.IsAnyLeaf() || current_node.GetGateStatus() == GateStatus::GATE_SET) {
			D_ASSERT(status == GateStatus::GATE_NOT_SET);
			D_ASSERT(current_key.Size() == key.len);
			if (!equal && current_key.Contains(key)) {
				return Next();
			}

			if (current_node.GetGateStatus() == GateStatus::GATE_SET) {
				FindMinimum(current_node);
			} else {
				last_leaf = current_node;
			}
			return true;
		}

		D_ASSERT(current_node.GetGateStatus() == GateStatus::GATE_NOT_SET);
		if (current_node.GetType() != NType::PREFIX) {
			auto next_byte = key[depth];
			auto child = current_node.GetNextChild(art, next_byte);

			// The key is greater than any key in this subtree.
			if (!child) {
				return Next();

Review comment (suggestion):
Lets use this as our loop-condition: `while (ref.get().HasMetadata())`... I am curious when we actually hit this, though 🤔 I.e., I would expect an empty child, instead of no metadata.

Code after:
		auto next = ref.get().GetNextChild(art, byte);
		D_ASSERT(next);

		// Move to the leftmost node.
		current_key.Push(byte);
		if (status == GateStatus::GATE_SET) {
			row_id[nested_depth] = byte;
			nested_depth++;
			D_ASSERT(nested_depth < Prefix::ROW_ID_SIZE);
		}
		nodes.emplace(ref.get(), byte);
		ref = *next;
	}
	// Should always have a node with metadata.
	throw InternalException("ART Iterator::FindMinimum: Reached node without metadata");
}

bool Iterator::LowerBound(const Node &node, const ARTKey &key, const bool equal) {
	reference<const Node> ref(node);
	idx_t depth = 0;

	while (ref.get().HasMetadata()) {
		// We found any leaf node, or a gate.
		if (ref.get().IsAnyLeaf() || ref.get().GetGateStatus() == GateStatus::GATE_SET) {
			D_ASSERT(status == GateStatus::GATE_NOT_SET);
			D_ASSERT(current_key.Size() == key.len);
			if (!equal && current_key.Contains(key)) {
				return Next();
			}

			if (ref.get().GetGateStatus() == GateStatus::GATE_SET) {
				FindMinimum(ref.get());
			} else {
				last_leaf = ref.get();
			}
			return true;
		}

		D_ASSERT(ref.get().GetGateStatus() == GateStatus::GATE_NOT_SET);
		if (ref.get().GetType() != NType::PREFIX) {
			auto next_byte = key[depth];
			auto child = ref.get().GetNextChild(art, next_byte);

			// The key is greater than any key in this subtree.
			if (!child) {
				return Next();
			}

			current_key.Push(next_byte);
			nodes.emplace(ref.get(), next_byte);
Repository: duckdb/duckdb
File: src/include/duckdb/storage/table/scan_state.hpp (C++)

Code before:
	bool do_system_sample = false;
	//! The sampling rate to use
	double sample_rate;
};

struct TableScanOptions {
	//! Fetch rows one-at-a-time instead of using the regular scans.
	bool force_fetch_row = false;
};

class CheckpointLock {
public:
	explicit CheckpointLock(unique_ptr<StorageLockKey> lock_p) : lock(std::move(lock_p)) {
	}

private:
	unique_ptr<StorageLockKey> lock;
};

class TableScanState {
public:
	TableScanState();
	~TableScanState();

	//! The underlying table scan state
	unique_ptr<CollectionScanState> table_state;
	//! Transaction-local scan state
	unique_ptr<CollectionScanState> local_state;
	//! Options for scanning
	TableScanOptions options;
	//! Shared lock over the checkpoint to prevent checkpoints while reading
	shared_ptr<CheckpointLock> checkpoint_lock;
	//! Filter info
	ScanFilterInfo filters;
	//! Sampling info
	ScanSamplingInfo sampling_info;

public:
	void Initialize(vector<StorageIndex> column_ids, optional_ptr<ClientContext> context = nullptr,
	                optional_ptr<TableFilterSet> table_filters = nullptr,
	                optional_ptr<SampleOptions> table_sampling = nullptr);

	const vector<StorageIndex> &GetColumnIds();

	ScanFilterInfo &GetFilterInfo();

	ScanSamplingInfo &GetSamplingInfo();

private:
	//! The column identifiers of the scan
	vector<StorageIndex> column_ids;

Review comment (suggestion):
This seems to be required because we are now using inheritance - but I would say that's not really required. Maybe we can just add a member to the scan state instead (e.g. a `shared_ptr<RowGroupReorderer>` or similar)? That should allow us to avoid most of the code changes here.

Code after:
	double sample_rate;
};

struct TableScanOptions {
	//! Fetch rows one-at-a-time instead of using the regular scans.
	bool force_fetch_row = false;
};

class CheckpointLock {
public:
	explicit CheckpointLock(unique_ptr<StorageLockKey> lock_p) : lock(std::move(lock_p)) {
	}

private:
	unique_ptr<StorageLockKey> lock;
};

class TableScanState {
public:
	TableScanState();
	~TableScanState();

	//! The underlying table scan state
	CollectionScanState table_state;
	//! Transaction-local scan state
	CollectionScanState local_state;
	//! Options for scanning
	TableScanOptions options;
	//! Shared lock over the checkpoint to prevent checkpoints while reading
	shared_ptr<CheckpointLock> checkpoint_lock;
	//! Filter info
	ScanFilterInfo filters;
	//! Sampling info
	ScanSamplingInfo sampling_info;

public:
	void Initialize(vector<StorageIndex> column_ids, optional_ptr<ClientContext> context = nullptr,
	                optional_ptr<TableFilterSet> table_filters = nullptr,
	                optional_ptr<SampleOptions> table_sampling = nullptr);

	const vector<StorageIndex> &GetColumnIds();

	ScanFilterInfo &GetFilterInfo();

	ScanSamplingInfo &GetSamplingInfo();

private:
	//! The column identifiers of the scan
	vector<StorageIndex> column_ids;
};
Repository: duckdb/duckdb
File: test/appender/test_appender.cpp (C++)

Code before:
		try {
			app.Flush();
		} catch (...) {
			// Expected when the query is interrupted
		}
	});

	// Wait until the flush thread starts, then interrupt
	while (!flush_started.load()) {
		this_thread::yield();
	}
	// Give the flush a tiny moment to get into execution before interrupting
	std::this_thread::sleep_for(std::chrono::milliseconds(50));
	con.Interrupt();

	t.join();

	// Now clear pending buffers so Close will not attempt to flush again
	app.Clear();

	// Should finish eventually. Close must complete quickly since no data remains to flush
	auto future = std::async(std::launch::async, [&]() {
	    app.Close();
	});

	auto status = future.wait_for(std::chrono::seconds(1));

	if (status == std::future_status::ready) {
	    REQUIRE_NOTHROW(future.get());
	} else {
		con.Interrupt();
	    FAIL("app.Close() did not finish within a second");
	}
}



Review comment (suggestion):
Maybe we can even lower the time here - if we cleared the data, `close` should be very swift!

Code after:

	// Append a single row so we actually have something to flush
	app.AppendRow(1);

	atomic<bool> flush_started {false};

	thread t([&]() {
		flush_started.store(true);
		try {
			app.Flush();
		} catch (std::exception &ex) {
			ErrorData error_data(ex);
			REQUIRE((error_data.Type() == ExceptionType::INTERRUPT));
		}
	});

	// Wait until the flush thread starts, then interrupt
	while (!flush_started.load()) {
		this_thread::yield();
	}
	// Give the flush a tiny moment to get into execution before interrupting
	std::this_thread::sleep_for(std::chrono::milliseconds(50));
	con.Interrupt();

	t.join();

	// Now clear pending buffers so Close will not attempt to flush again
	app.Clear();

	// Should finish eventually. Close must complete quickly since no data remains to flush
	auto future = std::async(std::launch::async, [&]() { app.Close(); });

	auto status = future.wait_for(std::chrono::milliseconds(50));

	if (status == std::future_status::ready) {
		REQUIRE_NOTHROW(future.get());
	} else {
		con.Interrupt();
		FAIL("app.Close() did not finish within a second");
	}
}
Repository: duckdb/duckdb
File: extension/core_functions/scalar/struct/struct_keys.cpp (C++)

Code before:
#include "duckdb/common/types/vector.hpp"
#include "duckdb/execution/expression_executor_state.hpp"
#include "core_functions/scalar/struct_functions.hpp"

namespace duckdb {

static void StructKeysFunction(DataChunk &args, ExpressionState &state, Vector &result) {
	auto &input = args.data[0];
	const idx_t count = args.size();

	if (input.GetType().id() != LogicalTypeId::STRUCT) {
		throw InvalidInputException("struct_keys() expects a STRUCT argument");
	}

	auto keys = StructVector::GetKeys(input);
	const idx_t key_count = keys.size();
	const auto list_type = LogicalType::LIST(LogicalType::VARCHAR);

	// If the input is a constant during constant folding, we must return a CONSTANT_VECTOR
	if (input.GetVectorType() == VectorType::CONSTANT_VECTOR) {
		if (ConstantVector::IsNull(input)) {
			Vector null_vec(list_type);
			ConstantVector::SetNull(null_vec, true);
			result.Reference(null_vec);
			result.SetVectorType(VectorType::CONSTANT_VECTOR);
			return;
		}

		Vector list_vec(list_type);
		auto &list_child = ListVector::GetEntry(list_vec);
		list_child.SetVectorType(VectorType::FLAT_VECTOR);
		auto child_data = FlatVector::GetData<string_t>(list_child);
		for (idx_t i = 0; i < key_count; i++) {
			child_data[i] = StringVector::AddString(list_child, keys[i]);
		}
		ListVector::SetListSize(list_vec, key_count);
		auto list_entries = FlatVector::GetData<list_entry_t>(list_vec);
		list_entries[0] = {0, key_count};

Review comment (suggestion):
I think we can remove these lines, as they should be handled earlier on during binding (no matching function found). Maybe you can try to trigger this error path in one of your tests, like so:

```sql
statement error
SELECT struct_keys(1);
----
you expected error message
```

Code after:
#include "duckdb/common/types/vector.hpp"
#include "duckdb/execution/expression_executor_state.hpp"
#include "core_functions/scalar/struct_functions.hpp"
#include "duckdb/planner/expression/bound_function_expression.hpp"

namespace duckdb {

struct StructKeysBindData : public FunctionData {
	const LogicalType type;
	Vector keys_vector;

	explicit StructKeysBindData(const LogicalType &type_p)
	    : type(type_p), keys_vector(LogicalType::LIST(LogicalType::VARCHAR), 2) {
		const auto &child_types = StructType::GetChildTypes(type);
		const auto count = child_types.size();

		ListVector::Reserve(keys_vector, count);
		auto &list_child = ListVector::GetEntry(keys_vector);
		auto child_data = FlatVector::GetData<string_t>(list_child);
		for (idx_t i = 0; i < count; i++) {
			child_data[i] = StringVector::AddString(list_child, child_types[i].first);
		}
		ListVector::SetListSize(keys_vector, count);

		auto list_entries = FlatVector::GetData<list_entry_t>(keys_vector);
		list_entries[0] = {0, count};

		auto &validity = FlatVector::Validity(keys_vector);
		validity.EnsureWritable();
		validity.SetInvalid(1);
	}

	bool Equals(const FunctionData &other) const override {
		auto &o = other.Cast<StructKeysBindData>();
		// Compare type and flag (content is derived from them)
		return type == o.type;
	}
Repository: duckdb/duckdb
File: extension/core_functions/scalar/struct/struct_keys.cpp (C++)

Code before:
	SelectionVector sel(count);
	UnifiedVectorFormat input_data;
	input.ToUnifiedFormat(count, input_data);
	for (idx_t i = 0; i < count; i++) {
		auto idx = input_data.sel->get_index(i);
		const bool is_valid = input_data.validity.RowIsValid(idx);
		sel.set_index(i, is_valid ? 0 : 1);
	}

	result.Reference(dict_child);
	result.Dictionary(2, sel, count);
}

static unique_ptr<FunctionData> StructKeysBind(ClientContext &context, ScalarFunction &bound_function,
                                               vector<unique_ptr<Expression>> &arguments) {
	if (arguments.size() != 1) {
		throw InvalidInputException("struct_keys() expects exactly one argument");
	}
	if (arguments[0]->return_type.id() != LogicalTypeId::STRUCT) {
		throw InvalidInputException("struct_keys() expects a STRUCT argument");
	}
	return nullptr;
}

ScalarFunction StructKeysFun::GetFunction() {
	ScalarFunction func({LogicalType::ANY}, LogicalType::LIST(LogicalType::VARCHAR), StructKeysFunction);
	func.bind = StructKeysBind;
	return func;
}

} // namespace duckdb


Review comment (suggestion):
I think you should be able to type this as 
```
ScalarFunction func({LogicalTypeId::STRUCT}, LogicalType::LIST(LogicalType::VARCHAR), StructKeysFunction);
```

The `LogicalTypeId::STRUCT` tells the binder that it needs to function needs to take a single struct-type argument, even if the exact struct isn't known, so you don't need to check for this again in `StructKeysBind`

Code after:
		auto idx = input_data.sel->get_index(i);
		const bool is_valid = input_data.validity.RowIsValid(idx);
		sel.set_index(i, !is_valid);
	}

	result.Slice(keys_vector, sel, count);
}

static unique_ptr<FunctionData> StructKeysBind(ClientContext &context, ScalarFunction &bound_function,
                                               vector<unique_ptr<Expression>> &arguments) {
	if (arguments[0]->return_type.id() != LogicalTypeId::STRUCT) {
		throw InvalidInputException("struct_keys() expects a STRUCT argument");
	}

	const bool is_unnamed = StructType::IsUnnamed(arguments[0]->return_type);
	if (is_unnamed) {
		throw InvalidInputException("struct_keys() cannot be applied to an unnamed STRUCT");
	}
	return make_uniq<StructKeysBindData>(arguments[0]->return_type);
}

ScalarFunction StructKeysFun::GetFunction() {
	ScalarFunction func({LogicalType::ANY}, LogicalType::LIST(LogicalType::VARCHAR), StructKeysFunction,
	                    StructKeysBind);
	return func;
}

} // namespace duckdb
Repository: duckdb/duckdb
File: src/storage/table/row_version_manager.cpp (C++)

Code before:
                                                             idx_t start) {
	if (!delete_pointer.IsValid()) {
		return nullptr;
	}
	auto version_info = make_shared_ptr<RowVersionManager>(start);
	MetadataReader source(manager, delete_pointer, &version_info->storage_pointers);
	auto chunk_count = source.Read<idx_t>();
	D_ASSERT(chunk_count > 0);
	for (idx_t i = 0; i < chunk_count; i++) {
		idx_t vector_index = source.Read<idx_t>();
		if (vector_index * STANDARD_VECTOR_SIZE >= Storage::MAX_ROW_GROUP_SIZE) {
			throw IOException("In DeserializeDeletes, vector_index %llu is out of range for the max row group size of "
			                  "%llu. Corrupted file?",
			                  vector_index, Storage::MAX_ROW_GROUP_SIZE);
		}

		version_info->FillVectorInfo(vector_index);
		version_info->vector_info[vector_index] = ChunkInfo::Read(source);
	}
	version_info->has_changes = false;
	return version_info;
}

bool RowVersionManager::HasChanges() {
	lock_guard<mutex> lock(version_lock);
	return has_changes;
}

vector<MetaBlockPointer> RowVersionManager::GetStoragePointers() {
	lock_guard<mutex> lock(version_lock);
	D_ASSERT(!has_changes);
	return storage_pointers;
}

} // namespace duckdb


Review comment (bug):
Correct me if I'm wrong - but this now signifies "there have been deletes made that have not yet been serialized" - is that correct? Maybe we can rename this function to `HasUnsavedChanges()` or `HasUnserializedChanges()` in that case for clarity? 

Code after:

shared_ptr<RowVersionManager> RowVersionManager::Deserialize(MetaBlockPointer delete_pointer, MetadataManager &manager,
                                                             idx_t start) {
	if (!delete_pointer.IsValid()) {
		return nullptr;
	}
	auto version_info = make_shared_ptr<RowVersionManager>(start);
	MetadataReader source(manager, delete_pointer, &version_info->storage_pointers);
	auto chunk_count = source.Read<idx_t>();
	D_ASSERT(chunk_count > 0);
	for (idx_t i = 0; i < chunk_count; i++) {
		idx_t vector_index = source.Read<idx_t>();
		if (vector_index * STANDARD_VECTOR_SIZE >= Storage::MAX_ROW_GROUP_SIZE) {
			throw IOException("In DeserializeDeletes, vector_index %llu is out of range for the max row group size of "
			                  "%llu. Corrupted file?",
			                  vector_index, Storage::MAX_ROW_GROUP_SIZE);
		}

		version_info->FillVectorInfo(vector_index);
		version_info->vector_info[vector_index] = ChunkInfo::Read(source);
	}
	version_info->has_unserialized_changes = false;
	return version_info;
}

bool RowVersionManager::HasUnserializedChanges() {
	lock_guard<mutex> lock(version_lock);
	return has_unserialized_changes;
}

vector<MetaBlockPointer> RowVersionManager::GetStoragePointers() {
	lock_guard<mutex> lock(version_lock);
	D_ASSERT(!has_unserialized_changes);
	return storage_pointers;
}

} // namespace duckdb
Repository: duckdb/duckdb
File: src/main/attached_database.cpp (C++)

Code before:
const Catalog &AttachedDatabase::ParentCatalog() const {
	return *parent_catalog;
}

bool AttachedDatabase::IsInitialDatabase() const {
	return is_initial_database;
}

void AttachedDatabase::SetInitialDatabase() {
	is_initial_database = true;
}

void AttachedDatabase::SetReadOnlyDatabase() {
	type = AttachedDatabaseType::READ_ONLY_DATABASE;
}

void AttachedDatabase::OnDetach(ClientContext &context) {
	if (catalog) {
		catalog->OnDetach(context);
	}
	if (stored_database_path && visibility != AttachVisibility::HIDDEN) {
		stored_database_path->OnDetach();
	}
}

void AttachedDatabase::Checkpoint() {
	if (!checkpoint) {
		return;
	}
	D_ASSERT(catalog);
	checkpoint = false;

	if (storage && !storage->InMemory() && !ValidChecker::IsInvalidated(db)) {
		auto &config = DBConfig::GetConfig(db);
		if (config.options.checkpoint_on_shutdown) {
			CheckpointOptions options;
			options.wal_action = CheckpointWALAction::DELETE_WAL;
			storage->CreateCheckpoint(QueryContext(), options);
		}
	}
}

void AttachedDatabase::Cleanup() {
	if (!cleanup) {
		return;
	}
	cleanup = false;

	transaction_manager.reset();
	catalog.reset();
	storage.reset();

Review comment (bug):
Are we ever calling `Checkpoint()` without closing the database?

Given that `Checkpoint` can also only ever be called once due to the `checkpoint` flag being set - I think this is a bit of a misnomer.

Maybe we can parameterize the `Close` instead, e.g.:

```cpp
Close(DatabaseCloseAction::CHECKPOINT); // throws if checkpoint fails, but still cleans up regardless
Close(DatabaseCloseAction::TRY_CHECKPOINT); // tries to checkpoint, if it fails does not throw
Close(DatabaseCloseAction::CLEANUP_ONLY); // does not try to checkpoint

```

Code after:
}

void AttachedDatabase::SetInitialDatabase() {
	is_initial_database = true;
}

void AttachedDatabase::SetReadOnlyDatabase() {
	type = AttachedDatabaseType::READ_ONLY_DATABASE;
}

void AttachedDatabase::OnDetach(ClientContext &context) {
	if (catalog) {
		catalog->OnDetach(context);
	}
	if (stored_database_path && visibility != AttachVisibility::HIDDEN) {
		stored_database_path->OnDetach();
	}
}

void AttachedDatabase::Close(const DatabaseCloseAction action) {
	if (is_closed) {
		return;
	}
	D_ASSERT(catalog);
	is_closed = true;

	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 {
				DUCKDB_LOG_ERROR(db, "Silent exception in AttachedDatabase::Close():\t" + data.Message());
			} catch (...) { // NOLINT
			}
		} else {
			Cleanup();
Repository: duckdb/duckdb
File: src/main/attached_database.cpp (C++)

Code before:
}

string AttachedDatabase::ExtractDatabaseName(const string &dbpath, FileSystem &fs) {
	if (dbpath.empty() || dbpath == IN_MEMORY_PATH) {
		return "memory";
	}
	auto name = RemoveQueryParams(fs.ExtractBaseName(dbpath));
	if (NameIsReserved(name)) {
		name += "_db";
	}
	return name;
}

void AttachedDatabase::Initialize(optional_ptr<ClientContext> context) {
	if (IsSystem()) {
		catalog->Initialize(context, true);
	} else {
		catalog->Initialize(context, false);
	}
	if (storage) {
		storage->Initialize(context);
	}
}

void AttachedDatabase::FinalizeLoad(optional_ptr<ClientContext> context) {
	catalog->FinalizeLoad(context);
}

bool AttachedDatabase::HasStorageManager() const {
	return storage.get();
}

StorageManager &AttachedDatabase::GetStorageManager() {
	if (!storage) {
		throw InternalException("Internal system catalog does not have storage");
	}
	return *storage;
}

Catalog &AttachedDatabase::GetCatalog() {
	return *catalog;
}

TransactionManager &AttachedDatabase::GetTransactionManager() {
	return *transaction_manager;
}

Catalog &AttachedDatabase::ParentCatalog() {
	return *parent_catalog;
}


Review comment (suggestion):
Shouldn't `attached_db.reset();` also be within the lock?
Otherwise two of those calls side by side (say those are the last two references) will get `use_count() == 2` the first time in the lock, and at the second time either `2` or `1` depending on timing of operations.

Code after:
	auto vec = StringUtil::Split(name, "?");
	D_ASSERT(!vec.empty());
	return vec[0];
}

string AttachedDatabase::ExtractDatabaseName(const string &dbpath, FileSystem &fs) {
	if (dbpath.empty() || dbpath == IN_MEMORY_PATH) {
		return "memory";
	}
	auto name = RemoveQueryParams(fs.ExtractBaseName(dbpath));
	if (NameIsReserved(name)) {
		name += "_db";
	}
	return name;
}

void AttachedDatabase::InvokeCloseIfLastReference(shared_ptr<AttachedDatabase> &attached_db) {
	{
		lock_guard<mutex> guard(attached_db->close_lock);
		if (attached_db.use_count() == 1) {
			attached_db->Close(DatabaseCloseAction::CHECKPOINT);
		}
	}
	attached_db.reset();
}

void AttachedDatabase::Initialize(optional_ptr<ClientContext> context) {
	if (IsSystem()) {
		catalog->Initialize(context, true);
	} else {
		catalog->Initialize(context, false);
	}
	if (storage) {
		storage->Initialize(context);
	}
}

void AttachedDatabase::FinalizeLoad(optional_ptr<ClientContext> context) {
	catalog->FinalizeLoad(context);
}

bool AttachedDatabase::HasStorageManager() const {
	return storage.get();
}

StorageManager &AttachedDatabase::GetStorageManager() {
	if (!storage) {
		throw InternalException("Internal system catalog does not have storage");
	}
	return *storage;
}