ronantakizawa/codereview-bench huggingface dataset:
Repository: duckdb/duckdb
File: test/sql/parallelism/interquery/concurrent_attach_detach.cpp (C++)

Code before:

	void addWorker(AttachWorker &worker, const idx_t i);
	void removeWorker(AttachWorker &worker, const idx_t i);

	DBInfo db_infos[db_count];
};

struct AttachWorker {
public:
	AttachWorker(DuckDB &db, idx_t worker_id, vector<string> &logs, DBPoolMgr &db_pool)
	    : conn(db), worker_id(worker_id), logs(logs), db_pool(db_pool) {
	}

public:
	unique_ptr<MaterializedQueryResult> execQuery(const string &query) {
		return ::execQuery(conn, query);
	}
	void Work();

private:
	AttachTask RandomTask();
	void createTbl(AttachTask &task);
	void lookup(AttachTask &task);
	void append_internal(AttachTask &task, const bool is_upsert);
	void append(AttachTask &task);
	void delete_internal(AttachTask &task);
	void apply_changes(AttachTask &task);
	void describe_tbl(AttachTask &task);
	void checkpoint_db(AttachTask &task);
	void GetRandomTable(AttachTask &task);
	void addLog(const string &msg) {
		logs.push_back(msg);
	}

public:
	Connection conn;
	idx_t worker_id;
	vector<string> &logs;
	DBPoolMgr &db_pool;
};

void DBPoolMgr::addWorker(AttachWorker &worker, const idx_t i) {
	lock_guard<mutex> lock(mu);

	if (m.find(i) != m.end()) {
		m[i]++;
		return;
	}
	m[i] = 1;

	string query = "ATTACH '" + getDBPath(i) + "'";

Review comment (bug):
```sh
In file included from /home/vscode/duckdb/build/debug/test/sql/parallelism/interquery/ub_test_sql_interquery_parallelism.cpp:2:
/home/vscode/duckdb/test/sql/parallelism/interquery/concurrent_attach_detach.cpp:326:6: warning: 'void {anonymous}::AttachWorker::delete_internal({anonymous}::AttachTask&)' defined but not used [-Wunused-function]
  326 | void AttachWorker::delete_internal(AttachTask &task) {
      |      ^~~~~~~~~~~~
```

Code after:

	void addWorker(AttachWorker &worker, const idx_t i);
	void removeWorker(AttachWorker &worker, const idx_t i);

	DBInfo db_infos[db_count];
};

struct AttachWorker {
public:
	AttachWorker(DuckDB &db, idx_t worker_id, vector<string> &logs, DBPoolMgr &db_pool)
	    : conn(db), worker_id(worker_id), logs(logs), db_pool(db_pool) {
	}

public:
	unique_ptr<MaterializedQueryResult> execQuery(const string &query) {
		return ::execQuery(conn, query);
	}
	void Work();

private:
	AttachTask RandomTask();
	void createTbl(AttachTask &task);
	void lookup(AttachTask &task);
	void append_internal(AttachTask &task, const bool is_upsert);
	void append(AttachTask &task);
	void apply_changes(AttachTask &task);
	void describe_tbl(AttachTask &task);
	void checkpoint_db(AttachTask &task);
	void GetRandomTable(AttachTask &task);
	void addLog(const string &msg) {
		logs.push_back(msg);
	}

public:
	Connection conn;
	idx_t worker_id;
	vector<string> &logs;
	DBPoolMgr &db_pool;
};

void DBPoolMgr::addWorker(AttachWorker &worker, const idx_t i) {
	lock_guard<mutex> lock(mu);

	if (m.find(i) != m.end()) {
		m[i]++;
		return;
	}
	m[i] = 1;

	string query = "ATTACH '" + getDBPath(i) + "'";
	worker.execQuery(query);
Repository: duckdb/duckdb
File: src/include/duckdb/common/lru_cache.hpp (C++)

Code before:
		lru_list.clear();
		current_memory = 0;
	}

	idx_t MaxMemory() const {
		return max_memory;
	}
	idx_t CurrentMemory() const {
		return current_memory;
	}
	size_t Size() const {
		return entry_map.size();
	}

private:
	struct Entry {
		shared_ptr<Val> value;
		idx_t memory;
		typename list<Key>::iterator lru_iterator;
	};

	using EntryMap = unordered_map<Key, Entry, KeyHash, KeyEqual>;

	void DeleteImpl(typename EntryMap::iterator iter) {
		current_memory -= iter->second.memory;
		D_ASSERT(current_memory >= 0);
		lru_list.erase(iter->second.lru_iterator);
		entry_map.erase(iter);
	}

	void EvictIfNeeded(idx_t required_memory) {
		if (max_memory == 0) {
			return;
		}

		// Evict LRU entries until we have enough space
		while (!lru_list.empty() && (current_memory + required_memory > max_memory)) {
			const auto &stale_key = lru_list.back();
			auto stale_it = entry_map.find(stale_key);
			if (stale_it != entry_map.end()) {
				DeleteImpl(stale_it);
			} else {
				// Should not happen, but be defensive
				lru_list.pop_back();
			}
		}
	}

	const idx_t max_memory;
	idx_t current_memory;
	EntryMap entry_map;

Review comment (performance):
```sh
/home/vscode/duckdb/src/include/duckdb/common/lru_cache.hpp:117:41: warning: comparison of unsigned expression in '>= 0' is always true [-Wtype-limits]
  117 |                 D_ASSERT(current_memory >= 0);
      |                          ~~~~~~~~~~~~~~~^~~~
```

Code after:
		lru_list.clear();
		current_memory = 0;
	}

	idx_t MaxMemory() const {
		return max_memory;
	}
	idx_t CurrentMemory() const {
		return current_memory;
	}
	size_t Size() const {
		return entry_map.size();
	}

private:
	struct Entry {
		shared_ptr<Val> value;
		idx_t memory;
		typename list<Key>::iterator lru_iterator;
	};

	using EntryMap = unordered_map<Key, Entry, KeyHash, KeyEqual>;

	void DeleteImpl(typename EntryMap::iterator iter) {
		current_memory -= iter->second.memory;
		lru_list.erase(iter->second.lru_iterator);
		entry_map.erase(iter);
	}

	void EvictIfNeeded(idx_t required_memory) {
		if (max_memory == 0) {
			return;
		}

		// Evict LRU entries until we have enough space
		while (!lru_list.empty() && (current_memory + required_memory > max_memory)) {
			const auto &stale_key = lru_list.back();
			auto stale_it = entry_map.find(stale_key);
			if (stale_it != entry_map.end()) {
				DeleteImpl(stale_it);
			} else {
				// Should not happen, but be defensive
				lru_list.pop_back();
			}
		}
	}

	const idx_t max_memory;
	idx_t current_memory;
	EntryMap entry_map;
	list<Key> lru_list;
Repository: duckdb/duckdb
File: src/storage/storage_info.cpp (C++)

Code before:
	{"v0.10.1", 64},
	{"v0.10.2", 64},
	{"v0.10.3", 64},
	{"v1.0.0", 64},
	{"v1.1.0", 64},
	{"v1.1.1", 64},
	{"v1.1.2", 64},
	{"v1.1.3", 64},
	{"v1.2.0", 65},
	{"v1.2.1", 65},
	{"v1.2.2", 65},
	{"v1.3.0", 66},
	{"v1.3.1", 66},
	{"v1.3.2", 66},
	{"v1.4.0", 67},
	{"v1.4.1", 67},
	{"v1.4.2", 67},
	{"v1.4.3", 67},
	{"v1.5.0", 67},
	{nullptr, 0}
};
// END OF STORAGE VERSION INFO
static_assert(DEFAULT_STORAGE_VERSION_INFO == VERSION_NUMBER, "Check on VERSION_INFO");

// START OF SERIALIZATION VERSION INFO
const uint64_t LATEST_SERIALIZATION_VERSION_INFO = 8;
const uint64_t DEFAULT_SERIALIZATION_VERSION_INFO = 1;
static const SerializationVersionInfo serialization_version_info[] = {
	{"v0.10.0", 1},
	{"v0.10.1", 1},
	{"v0.10.2", 1},
	{"v0.10.3", 2},
	{"v1.0.0", 2},
	{"v1.1.0", 3},
	{"v1.1.1", 3},
	{"v1.1.2", 3},
	{"v1.1.3", 3},
	{"v1.2.0", 4},
	{"v1.2.1", 4},
	{"v1.2.2", 4},
	{"v1.3.0", 5},
	{"v1.3.1", 5},
	{"v1.3.2", 5},
	{"v1.4.0", 6},
	{"v1.4.1", 6},
	{"v1.4.2", 6},
	{"v1.4.3", 6},
	{"v1.5.0", 7},
	{"latest", 7},
	{nullptr, 0}
};

Review comment (suggestion):
bump the version as now we serialise `AggregateStateTypeInfo` differently

Code after:
	{"v0.10.1", 64},
	{"v0.10.2", 64},
	{"v0.10.3", 64},
	{"v1.0.0", 64},
	{"v1.1.0", 64},
	{"v1.1.1", 64},
	{"v1.1.2", 64},
	{"v1.1.3", 64},
	{"v1.2.0", 65},
	{"v1.2.1", 65},
	{"v1.2.2", 65},
	{"v1.3.0", 66},
	{"v1.3.1", 66},
	{"v1.3.2", 66},
	{"v1.4.0", 67},
	{"v1.4.1", 67},
	{"v1.4.2", 67},
	{"v1.4.3", 67},
	{"v1.5.0", 67},
	{nullptr, 0}
};
// END OF STORAGE VERSION INFO
static_assert(DEFAULT_STORAGE_VERSION_INFO == VERSION_NUMBER, "Check on VERSION_INFO");

// START OF SERIALIZATION VERSION INFO
const uint64_t LATEST_SERIALIZATION_VERSION_INFO = 7;
const uint64_t DEFAULT_SERIALIZATION_VERSION_INFO = 1;
static const SerializationVersionInfo serialization_version_info[] = {
	{"v0.10.0", 1},
	{"v0.10.1", 1},
	{"v0.10.2", 1},
	{"v0.10.3", 2},
	{"v1.0.0", 2},
	{"v1.1.0", 3},
	{"v1.1.1", 3},
	{"v1.1.2", 3},
	{"v1.1.3", 3},
	{"v1.2.0", 4},
	{"v1.2.1", 4},
	{"v1.2.2", 4},
	{"v1.3.0", 5},
	{"v1.3.1", 5},
	{"v1.3.2", 5},
	{"v1.4.0", 6},
	{"v1.4.1", 6},
	{"v1.4.2", 6},
	{"v1.4.3", 6},
	{"v1.5.0", 7},
	{"latest", 7},
	{nullptr, 0}
};
Repository: duckdb/duckdb
File: src/storage/storage_info.cpp (C++)

Code before:

// START OF SERIALIZATION VERSION INFO
const uint64_t LATEST_SERIALIZATION_VERSION_INFO = 8;
const uint64_t DEFAULT_SERIALIZATION_VERSION_INFO = 1;
static const SerializationVersionInfo serialization_version_info[] = {
	{"v0.10.0", 1},
	{"v0.10.1", 1},
	{"v0.10.2", 1},
	{"v0.10.3", 2},
	{"v1.0.0", 2},
	{"v1.1.0", 3},
	{"v1.1.1", 3},
	{"v1.1.2", 3},
	{"v1.1.3", 3},
	{"v1.2.0", 4},
	{"v1.2.1", 4},
	{"v1.2.2", 4},
	{"v1.3.0", 5},
	{"v1.3.1", 5},
	{"v1.3.2", 5},
	{"v1.4.0", 6},
	{"v1.4.1", 6},
	{"v1.4.2", 6},
	{"v1.4.3", 6},
	{"v1.5.0", 7},
	{"v1.6.0", 8},
	{"latest", 8},
	{nullptr, 0}
};
// END OF SERIALIZATION VERSION INFO
// clang-format on

static_assert(DEFAULT_SERIALIZATION_VERSION_INFO <= LATEST_SERIALIZATION_VERSION_INFO,
              "Check on SERIALIZATION_VERSION_INFO");

string GetStorageVersionName(const idx_t serialization_version, const bool add_suffix) {
	if (serialization_version < 4) {
		// special handling for lower serialization versions
		return "v1.0.0+";
	}
	optional_idx min_idx;
	for (idx_t i = 0; serialization_version_info[i].version_name; i++) {
		if (strcmp(serialization_version_info[i].version_name, "latest") == 0) {
			continue;
		}
		if (serialization_version_info[i].serialization_version != serialization_version) {
			continue;
		}
		if (!min_idx.IsValid()) {
			min_idx = i;
		}

Review comment (suggestion):
@lnkuiper I can see that the CI doesn't let me not adding this to the array.
How can I know what duckdb version is going to include this new code?
I assume that `v1.6.0` is not correct here.

Code after:

// START OF SERIALIZATION VERSION INFO
const uint64_t LATEST_SERIALIZATION_VERSION_INFO = 7;
const uint64_t DEFAULT_SERIALIZATION_VERSION_INFO = 1;
static const SerializationVersionInfo serialization_version_info[] = {
	{"v0.10.0", 1},
	{"v0.10.1", 1},
	{"v0.10.2", 1},
	{"v0.10.3", 2},
	{"v1.0.0", 2},
	{"v1.1.0", 3},
	{"v1.1.1", 3},
	{"v1.1.2", 3},
	{"v1.1.3", 3},
	{"v1.2.0", 4},
	{"v1.2.1", 4},
	{"v1.2.2", 4},
	{"v1.3.0", 5},
	{"v1.3.1", 5},
	{"v1.3.2", 5},
	{"v1.4.0", 6},
	{"v1.4.1", 6},
	{"v1.4.2", 6},
	{"v1.4.3", 6},
	{"v1.5.0", 7},
	{"latest", 7},
	{nullptr, 0}
};
// END OF SERIALIZATION VERSION INFO
// clang-format on

static_assert(DEFAULT_SERIALIZATION_VERSION_INFO <= LATEST_SERIALIZATION_VERSION_INFO,
              "Check on SERIALIZATION_VERSION_INFO");

string GetStorageVersionName(const idx_t serialization_version, const bool add_suffix) {
	if (serialization_version < 4) {
		// special handling for lower serialization versions
		return "v1.0.0+";
	}
	optional_idx min_idx;
	for (idx_t i = 0; serialization_version_info[i].version_name; i++) {
		if (strcmp(serialization_version_info[i].version_name, "latest") == 0) {
			continue;
		}
		if (serialization_version_info[i].serialization_version != serialization_version) {
			continue;
		}
		if (!min_idx.IsValid()) {
			min_idx = i;
		}
	}
Repository: duckdb/duckdb
File: src/include/duckdb/execution/index/art/iterator.hpp (C++)

Code before:
	}

private:
	//! The ART.
	ART &art;
	//! Stack of nodes from the root to the currently active node.
	stack<IteratorEntry> nodes;
	//! Last visited leaf node.
	Node last_leaf = Node();
	//! Holds the row ID of nested leaves.
	uint8_t row_id[ROW_ID_SIZE];
	//! True, if we passed a gate.
	GateStatus status;
	//! Depth in a nested leaf.
	uint8_t nested_depth = 0;
	//! True, if we entered a nested leaf to retrieve the next node.
	bool entered_nested_leaf = false;

	//! State for resuming within a leaf after early return due to max_count.
	//! For LEAF: cached row IDs and current position.
	set<row_t> cached_row_ids;
	set<row_t>::iterator cached_row_ids_it;
	bool has_cached_row_ids = false;
	//! For nested leaves: current byte position and whether we've started.
	uint8_t nested_byte = 0;
	bool nested_started = false;

private:
	//! Goes to the next leaf in the ART and sets it as last_leaf,
	//! returns false if there is no next leaf.
	bool Next();
	//! Pop the top node from the stack of iterator entries and adjust the current key.
	void PopNode();
};
} // namespace duckdb


Review comment (refactor):
Can we move this into a separate `struct` (can even be inside this class). Something like `ResumeScanState`?

Code after:
	//! Returns the nested depth.
	uint8_t GetNestedDepth() const {
		return nested_depth;
	}

private:
	//! The ART.
	ART &art;
	//! Stack of nodes from the root to the currently active node.
	stack<IteratorEntry> nodes;
	//! Last visited leaf node.
	Node last_leaf = Node();
	//! Holds the row ID of nested leaves.
	uint8_t row_id[ROW_ID_SIZE];
	//! True, if we passed a gate.
	GateStatus status;
	//! Depth in a nested leaf.
	uint8_t nested_depth = 0;
	//! True, if we entered a nested leaf to retrieve the next node.
	bool entered_nested_leaf = false;

	//! State for resuming a scan after early return due to the Output policy capacity (see note for the ARTScanResult
	//! enum).
	struct ResumeScanState {
		//! For LEAF: cached row IDs and current position.
		set<row_t> cached_row_ids;
		set<row_t>::iterator cached_row_ids_it;
		bool has_cached_row_ids = false;
		//! For nested leaves: current byte position and whether we've started.
		uint8_t nested_byte = 0;
		bool nested_started = false;
	};
	ResumeScanState resume_state;

private:
	//! Goes to the next leaf in the ART and sets it as last_leaf,
	//! returns false if there is no next leaf.
	bool Next();
	//! Pop the top node from the stack of iterator entries and adjust the current key.
	void PopNode();
};
} // namespace duckdb
Repository: duckdb/duckdb
File: src/include/duckdb/execution/index/art/iterator.hpp (C++)

Code before:
	}

	//! Returns true, if key_bytes contains all bytes of key.
	bool Contains(const ARTKey &key) const;
	//! Returns true, if key_bytes is greater than [or equal to] the key.
	bool GreaterThan(const ARTKey &key, const bool equal, const uint8_t nested_depth) const;

private:
	unsafe_vector<uint8_t> key_bytes;
};

//===--------------------------------------------------------------------===//
// Scan Output Policies
//===--------------------------------------------------------------------===//

//! Output policy for scanning row IDs only into a set.
struct RowIdSetOutput {
	set<row_t> &row_ids;

	explicit RowIdSetOutput(set<row_t> &row_ids) : row_ids(row_ids) {
	}

	bool IsFull(idx_t max_count) const {
#ifdef DEBUG
		D_ASSERT(row_ids.size() >= 0 && row_ids.size() <= max_count);
#endif
		return row_ids.size() >= max_count;
	}
	void SetKey(const IteratorKey &, idx_t) {
		// No-op: we don't need keys for row ID output.
	}
	void Emit(row_t rid) {
		row_ids.insert(rid);
	}
};

//! Output policy for scanning keys and row IDs into vectors.
struct KeyRowIdVectorOutput {
	ArenaAllocator &arena;
	unsafe_vector<ARTKey> &keys;
	unsafe_vector<ARTKey> &row_id_keys;
	idx_t count = 0;
	const_data_ptr_t key_data = nullptr;
	idx_t key_len = 0;

	KeyRowIdVectorOutput(ArenaAllocator &arena, unsafe_vector<ARTKey> &keys, unsafe_vector<ARTKey> &row_id_keys)
	    : arena(arena), keys(keys), row_id_keys(row_id_keys) {
	}

	idx_t Count() const {
		return count;

Review comment (bug):
`D_ASSERT` is likely good enough here and we can remove the `DEBUG` guard.

Code after:
	}

	//! Returns true, if key_bytes contains all bytes of key.
	bool Contains(const ARTKey &key) const;
	//! Returns true, if key_bytes is greater than [or equal to] the key.
	bool GreaterThan(const ARTKey &key, const bool equal, const uint8_t nested_depth) const;

private:
	unsafe_vector<uint8_t> key_bytes;
};

//===--------------------------------------------------------------------===//
// Scan Output Policies
//===--------------------------------------------------------------------===//

//! Output policy for scanning row IDs only into a set.
struct RowIdSetOutput {
	set<row_t> &row_ids;
	const idx_t capacity;

	RowIdSetOutput(set<row_t> &row_ids, const idx_t capacity) : row_ids(row_ids), capacity(capacity) {
	}

	bool IsFull() const {
		D_ASSERT(row_ids.size() >= 0 && row_ids.size() <= capacity);
		return row_ids.size() >= capacity;
	}
	void SetKey(const IteratorKey &, const idx_t) {
		// No-op: we don't need keys for row ID output.
	}
	void Add(const row_t rid) {
		row_ids.insert(rid);
	}
};

//! Output policy for scanning keys and row IDs.
struct KeyRowIdOutput {
	ArenaAllocator &arena;
	unsafe_vector<ARTKey> &keys;
	unsafe_vector<ARTKey> &row_id_keys;
	const idx_t capacity;
	idx_t count = 0;
	const_data_ptr_t key_data = nullptr;
	idx_t key_len = 0;

	KeyRowIdOutput(ArenaAllocator &arena, unsafe_vector<ARTKey> &keys, unsafe_vector<ARTKey> &row_id_keys,
	               const idx_t capacity)
	    : arena(arena), keys(keys), row_id_keys(row_id_keys), capacity(capacity) {
	}

	idx_t Count() const {
Repository: duckdb/duckdb
File: src/include/duckdb/common/lru_cache.hpp (C++)

Code before:
		return current_memory;
	}
	size_t Size() const {
		return entry_map.size();
	}
	bool IsEmpty() const {
		return entry_map.empty();
	}

private:
	struct Entry {
		shared_ptr<Val> value;
		idx_t memory;
		typename list<Key>::iterator lru_iterator;
		// Record memory reservation in the buffer pool, which is used for global memory control.
		unique_ptr<Payload> payload;
		idx_t payload_size;
	};

	using EntryMap = unordered_map<Key, Entry, KeyHash, KeyEqual>;

	void DeleteImpl(typename EntryMap::iterator iter) {
		current_memory -= iter->second.payload_size;
		D_ASSERT(current_memory >= 0);
		Cleanup cleanup;
		cleanup(iter->second.payload);
		lru_list.erase(iter->second.lru_iterator);
		entry_map.erase(iter);
	}

	void EvictIfNeeded(idx_t required_memory) {
		if (max_memory == 0) {
			return;
		}

		// Evict LRU entries until we have enough space
		while (!lru_list.empty() && (current_memory + required_memory > max_memory)) {
			const auto &stale_key = lru_list.back();
			auto stale_it = entry_map.find(stale_key);
			if (stale_it != entry_map.end()) {
				DeleteImpl(stale_it);
			} else {
				// Should not happen, but be defensive
				lru_list.pop_back();
			}
		}
	}

	const idx_t max_memory;
	idx_t current_memory;
	EntryMap entry_map;

Review comment (suggestion):
wondering do you think it proper to let `Payload` destructor to do cleanup?

A few benefits:
- Users don't need to understand whether `Cleanup` is used for `Val` or `Payload`
- We don't need to call `Cleanup` manually (RAII)
- No `Cleanup` instance creation

Code after:
	}

	idx_t Capacity() const {
		return max_total_weight;
	}
	idx_t CurrentTotalWeight() const {
		return current_total_weight;
	}
	size_t Size() const {
		return entry_map.size();
	}
	bool IsEmpty() const {
		return entry_map.empty();
	}

private:
	struct Entry {
		shared_ptr<Val> value;
		idx_t memory;
		typename list<Key>::iterator lru_iterator;
		// Record payload weight, which is used for global weight control.
		unique_ptr<Payload> payload;
		idx_t payload_weight;
	};

	using EntryMap = unordered_map<Key, Entry, KeyHash, KeyEqual>;

	void DeleteImpl(typename EntryMap::iterator iter) {
		current_total_weight -= iter->second.payload_weight;
		D_ASSERT(current_total_weight >= 0);
		lru_list.erase(iter->second.lru_iterator);
		entry_map.erase(iter);
	}

	void EvictIfNeeded(idx_t required_weight) {
		if (max_total_weight == 0) {
			return;
		}

		// Evict LRU entries until we have enough space
		while (!lru_list.empty() && (current_total_weight + required_weight > max_total_weight)) {
			const auto &stale_key = lru_list.back();
			auto stale_it = entry_map.find(stale_key);
			if (stale_it != entry_map.end()) {
				DeleteImpl(stale_it);
			} else {
				// Should not happen, but be defensive
				lru_list.pop_back();
			}
		}
	}
Repository: duckdb/duckdb
File: src/planner/operator/logical_delete.cpp (C++)

Code before:
                                                 table_info->Cast<CreateTableInfo>().table)) {
	auto binder = Binder::CreateBinder(context);
	bound_constraints = binder->BindConstraints(table);
}

idx_t LogicalDelete::EstimateCardinality(ClientContext &context) {
	return return_chunk ? LogicalOperator::EstimateCardinality(context) : 1;
}

vector<idx_t> LogicalDelete::GetTableIndex() const {
	return vector<idx_t> {table_index};
}

vector<ColumnBinding> LogicalDelete::GetColumnBindings() {
	if (return_chunk) {
		// Include table columns + rowid
		return GenerateColumnBindings(table_index, table.GetTypes().size() + 1);
	}
	return {ColumnBinding(0, 0)};
}

void LogicalDelete::ResolveTypes() {
	if (return_chunk) {
		types = table.GetTypes();
		// Add rowid as the last column for RETURNING
		types.push_back(LogicalType::ROW_TYPE);
	} else {
		types.emplace_back(LogicalType::BIGINT);
	}
}

string LogicalDelete::GetName() const {
#ifdef DEBUG
	if (DBConfigOptions::debug_print_bindings) {
		return LogicalOperator::GetName() + StringUtil::Format(" #%llu", table_index);
	}
#endif
	return LogicalOperator::GetName();
}

} // namespace duckdb


Review comment (suggestion):
I think this should do:
```C++
for (auto &col : table.GetVirtualColumns()) {
    types.push_back(col.type);
}
```
Although I am not 100% sure about in which order the columns arrive if a table has multiple virtual columns, but maybe that's a concern for later.

Code after:
                                                 table_info->Cast<CreateTableInfo>().table)) {
	auto binder = Binder::CreateBinder(context);
	bound_constraints = binder->BindConstraints(table);
}

idx_t LogicalDelete::EstimateCardinality(ClientContext &context) {
	return return_chunk ? LogicalOperator::EstimateCardinality(context) : 1;
}

vector<idx_t> LogicalDelete::GetTableIndex() const {
	return vector<idx_t> {table_index};
}

vector<ColumnBinding> LogicalDelete::GetColumnBindings() {
	if (return_chunk) {
		// Include table columns + virtual columns (e.g., rowid)
		auto virtual_columns = table.GetVirtualColumns();
		return GenerateColumnBindings(table_index, table.GetTypes().size() + virtual_columns.size());
	}
	return {ColumnBinding(0, 0)};
}

void LogicalDelete::ResolveTypes() {
	if (return_chunk) {
		types = table.GetTypes();
		// Add virtual columns (e.g., rowid)
		auto virtual_columns = table.GetVirtualColumns();
		for (auto &entry : virtual_columns) {
			types.push_back(entry.second.type);
		}
	} else {
		types.emplace_back(LogicalType::BIGINT);
	}
}

string LogicalDelete::GetName() const {
#ifdef DEBUG
	if (DBConfigOptions::debug_print_bindings) {
		return LogicalOperator::GetName() + StringUtil::Format(" #%llu", table_index);
	}
#endif
	return LogicalOperator::GetName();
}

} // namespace duckdb