Optimize DELETE by scanning indexed columns at bind time (duckdb#20682)

lnkuiper · web-flow · commit c87733c87b41 · 2026-01-29T12:01:42.000+01:00
diff --git a/src/execution/operator/persistent/physical_delete.cpp b/src/execution/operator/persistent/physical_delete.cpp
@@ -77,68 +77,21 @@ SinkResultType PhysicalDelete::Sink(ExecutionContext &context, DataChunk &chunk,
 	l_state.delete_chunk.Reset();
 	row_ids.Flatten(chunk.size());
 
-	// Check if we can use columns from the input chunk (passed through from the scan)
-	// instead of fetching them by row ID
-	bool use_input_columns = !return_columns.empty();
-
-	if (use_input_columns) {
-		// Use columns from the input chunk - they were passed through from the scan
-		// Only physical columns are passed; generated columns are computed in the RETURNING projection
-		for (idx_t i = 0; i < table.ColumnCount(); i++) {
-			D_ASSERT(return_columns[i] != DConstants::INVALID_INDEX);
+	// Use columns from the input chunk - they were passed through from the scan
+	// return_columns maps storage_idx -> chunk_idx
+	// For RETURNING: all columns have valid indices
+	// For index-only: only indexed columns have valid indices (sparse mapping)
+	D_ASSERT(!return_columns.empty() && "return_columns should always be populated for RETURNING or unique indexes");
+	for (idx_t i = 0; i < table.ColumnCount(); i++) {
+		if (return_columns[i] != DConstants::INVALID_INDEX) {
+			// Column was passed through from the scan
 			l_state.delete_chunk.data[i].Reference(chunk.data[return_columns[i]]);
-		}
-		l_state.delete_chunk.SetCardinality(chunk.size());
-	} else {
-		// Fall back to fetching columns by row ID
-		// This path is only used when unique indexes exist but no RETURNING
-		// (need indexed columns for delete tracking)
-		D_ASSERT(!return_chunk && "RETURNING should always use the optimized path with return_columns");
-
-		auto &transaction = DuckTransaction::Get(context.client, table.db);
-		auto to_be_fetched = vector<bool>(types.size(), false);
-		vector<StorageIndex> column_ids;
-		vector<LogicalType> column_types;
-
-		// Fetch only the required columns for updating the delete indexes
-		auto &local_storage = LocalStorage::Get(context.client, table.db);
-		auto storage = local_storage.GetStorage(table);
-		unordered_set<column_t> indexed_column_id_set;
-		storage->delete_indexes.Scan([&](Index &index) {
-			if (!index.IsBound() || !index.IsUnique()) {
-				return false;
-			}
-			auto &set = index.GetColumnIdSet();
-			indexed_column_id_set.insert(set.begin(), set.end());
-			return false;
-		});
-		for (auto &col : indexed_column_id_set) {
-			column_ids.emplace_back(col);
-		}
-		sort(column_ids.begin(), column_ids.end());
-		for (auto &col : column_ids) {
-			auto i = col.GetPrimaryIndex();
-			to_be_fetched[i] = true;
-			column_types.push_back(types[i]);
-		}
-
-		// Fetch the to-be-deleted chunk.
-		DataChunk fetch_chunk;
-		fetch_chunk.Initialize(Allocator::Get(context.client), column_types, chunk.size());
-		auto fetch_state = ColumnFetchState();
-		table.Fetch(transaction, fetch_chunk, column_ids, row_ids, chunk.size(), fetch_state);
-
-		// Reference the necessary columns of the fetch_chunk.
-		idx_t fetch_idx = 0;
-		for (idx_t i = 0; i < table.ColumnCount(); i++) {
-			if (to_be_fetched[i]) {
-				l_state.delete_chunk.data[i].Reference(fetch_chunk.data[fetch_idx++]);
-				continue;
-			}
+		} else {
+			// Column not in scan (sparse mapping for index-only case) - use NULL placeholder
 			l_state.delete_chunk.data[i].Reference(Value(types[i]));
 		}
-		l_state.delete_chunk.SetCardinality(fetch_chunk);
 	}
+	l_state.delete_chunk.SetCardinality(chunk.size());
 
 	// Append the deleted row IDs to the delete indexes.
 	// If we only delete local row IDs, then the delete_chunk is empty.
diff --git a/src/include/duckdb/planner/binder.hpp b/src/include/duckdb/planner/binder.hpp
@@ -408,6 +408,13 @@ class Binder : public enable_shared_from_this<Binder> {
 	void BindDeleteReturningColumns(TableCatalogEntry &table, LogicalGet &get, vector<idx_t> &return_columns,
 	                                vector<unique_ptr<Expression>> &projection_expressions,
 	                                LogicalOperator &target_binding);
+	//! Build a sparse mapping for unique index columns only (for DELETE without RETURNING)
+	//! return_columns[storage_idx] = scan_chunk_idx (only for indexed columns)
+	void BindDeleteIndexColumns(TableCatalogEntry &table, LogicalGet &get, vector<idx_t> &return_columns);
+	//! Overload for MERGE INTO: builds projection expressions and maps storage_idx -> projection_expr_idx
+	void BindDeleteIndexColumns(TableCatalogEntry &table, LogicalGet &get, vector<idx_t> &return_columns,
+	                            vector<unique_ptr<Expression>> &projection_expressions,
+	                            LogicalOperator &target_binding);
 	BoundStatement BindReturning(vector<unique_ptr<ParsedExpression>> returning_list, TableCatalogEntry &table,
 	                             const string &alias, idx_t update_table_index,
 	                             unique_ptr<LogicalOperator> child_operator,
diff --git a/src/planner/binder.cpp b/src/planner/binder.cpp
@@ -403,33 +403,108 @@ void Binder::BindDeleteReturningColumns(TableCatalogEntry &table, LogicalGet &ge
 	}
 }
 
-void Binder::BindDeleteReturningColumns(TableCatalogEntry &table, LogicalGet &get, vector<idx_t> &return_columns,
-                                        vector<unique_ptr<Expression>> &projection_expressions,
-                                        LogicalOperator &target_binding) {
-	// First, use the base helper to ensure all physical columns are in the scan
-	vector<idx_t> scan_return_columns;
-	BindDeleteReturningColumns(table, get, scan_return_columns);
-
-	// Resolve types after adding columns to the scan
+//! Helper: convert scan column mapping to projection expression mapping for MERGE INTO
+static void ConvertScanToProjectionMapping(TableCatalogEntry &table, const vector<idx_t> &scan_return_columns,
+                                           vector<idx_t> &return_columns,
+                                           vector<unique_ptr<Expression>> &projection_expressions,
+                                           LogicalOperator &target_binding) {
 	target_binding.ResolveOperatorTypes();
 	auto target_bindings = target_binding.GetColumnBindings();
 	auto &target_types = target_binding.types;
 
-	// Convert scan mapping to projection expression mapping
-	// return_columns[storage_idx] = projection_expr_idx
 	auto physical_count = table.GetColumns().PhysicalColumnCount();
 	return_columns.resize(physical_count, DConstants::INVALID_INDEX);
 
 	for (idx_t storage_idx = 0; storage_idx < scan_return_columns.size(); storage_idx++) {
 		auto scan_idx = scan_return_columns[storage_idx];
 		if (scan_idx != DConstants::INVALID_INDEX && scan_idx < target_bindings.size()) {
 			return_columns[storage_idx] = projection_expressions.size();
-			auto col_ref = make_uniq<BoundColumnRefExpression>(target_types[scan_idx], target_bindings[scan_idx]);
-			projection_expressions.push_back(std::move(col_ref));
+			projection_expressions.push_back(
+			    make_uniq<BoundColumnRefExpression>(target_types[scan_idx], target_bindings[scan_idx]));
 		}
 	}
 }
 
+void Binder::BindDeleteReturningColumns(TableCatalogEntry &table, LogicalGet &get, vector<idx_t> &return_columns,
+                                        vector<unique_ptr<Expression>> &projection_expressions,
+                                        LogicalOperator &target_binding) {
+	vector<idx_t> scan_return_columns;
+	BindDeleteReturningColumns(table, get, scan_return_columns);
+	ConvertScanToProjectionMapping(table, scan_return_columns, return_columns, projection_expressions, target_binding);
+}
+
+void Binder::BindDeleteIndexColumns(TableCatalogEntry &table, LogicalGet &get, vector<idx_t> &return_columns) {
+	// Build a mapping from storage column index to scan chunk index for unique index tracking.
+	// This is a sparse mapping - only indexed columns have valid indices.
+	// Used when DELETE has no RETURNING but table has unique indexes.
+	auto &storage = table.GetStorage();
+	auto &info = storage.GetDataTableInfo();
+	auto &indexes = info->GetIndexes();
+
+	// Collect column IDs from unique indexes
+	unordered_set<column_t> indexed_column_ids;
+	indexes.Scan([&](Index &index) {
+		if (index.IsUnique()) {
+			auto &col_ids = index.GetColumnIdSet();
+			indexed_column_ids.insert(col_ids.begin(), col_ids.end());
+		}
+		return false;
+	});
+
+	if (indexed_column_ids.empty()) {
+		return;
+	}
+
+	auto &column_ids = get.GetColumnIds();
+	auto &columns = table.GetColumns();
+	auto physical_count = columns.PhysicalColumnCount();
+
+	// Initialize the mapping with INVALID_INDEX
+	return_columns.resize(physical_count, DConstants::INVALID_INDEX);
+
+	// First, map columns already in the scan to their storage indices
+	for (idx_t chunk_idx = 0; chunk_idx < column_ids.size(); chunk_idx++) {
+		auto &col_id = column_ids[chunk_idx];
+		if (col_id.IsVirtualColumn()) {
+			continue;
+		}
+		auto logical_idx = col_id.GetPrimaryIndex();
+		auto &col = columns.GetColumn(LogicalIndex(logical_idx));
+		if (!col.Generated()) {
+			auto storage_idx = col.StorageOid();
+			// Only map if this column is in a unique index
+			if (indexed_column_ids.count(storage_idx)) {
+				return_columns[storage_idx] = chunk_idx;
+			}
+		}
+	}
+
+	// Add any missing indexed columns to the scan
+	for (auto col_idx : indexed_column_ids) {
+		if (return_columns[col_idx] == DConstants::INVALID_INDEX) {
+			return_columns[col_idx] = column_ids.size();
+			// Find the logical index for this storage index
+			for (auto &col : columns.Physical()) {
+				if (col.StorageOid() == col_idx) {
+					get.AddColumnId(col.Logical().index);
+					break;
+				}
+			}
+		}
+	}
+}
+
+void Binder::BindDeleteIndexColumns(TableCatalogEntry &table, LogicalGet &get, vector<idx_t> &return_columns,
+                                    vector<unique_ptr<Expression>> &projection_expressions,
+                                    LogicalOperator &target_binding) {
+	vector<idx_t> scan_return_columns;
+	BindDeleteIndexColumns(table, get, scan_return_columns);
+	if (!scan_return_columns.empty()) {
+		ConvertScanToProjectionMapping(table, scan_return_columns, return_columns, projection_expressions,
+		                               target_binding);
+	}
+}
+
 BoundStatement Binder::BindReturning(vector<unique_ptr<ParsedExpression>> returning_list, TableCatalogEntry &table,
                                      const string &alias, idx_t update_table_index,
                                      unique_ptr<LogicalOperator> child_operator, virtual_column_map_t virtual_columns) {
diff --git a/src/planner/binder/statement/bind_delete.cpp b/src/planner/binder/statement/bind_delete.cpp
@@ -7,6 +7,7 @@
 #include "duckdb/planner/operator/logical_get.hpp"
 #include "duckdb/planner/operator/logical_cross_product.hpp"
 #include "duckdb/catalog/catalog_entry/table_catalog_entry.hpp"
+#include "duckdb/storage/data_table.hpp"
 
 namespace duckdb {
 
@@ -64,11 +65,18 @@ BoundStatement Binder::Bind(DeleteStatement &stmt) {
 	auto del = make_uniq<LogicalDelete>(table, GenerateTableIndex());
 	del->bound_constraints = BindConstraints(table);
 
-	// If RETURNING is present, add all physical table columns to the scan so we can pass them through
-	// instead of having to fetch them by row ID in PhysicalDelete.
-	// Generated columns will be computed in the RETURNING projection by the binder.
+	// Add columns to the scan to avoid fetching by row ID in PhysicalDelete:
+	// - If RETURNING: add all physical columns (for RETURNING projection)
+	// - Else if unique indexes exist: add only indexed columns (for delete index tracking)
 	if (!stmt.returning_list.empty()) {
+		// Add all physical columns for RETURNING
 		BindDeleteReturningColumns(table, get, del->return_columns);
+	} else if (table.IsDuckTable()) {
+		// Only optimize for DuckDB tables (not attached external tables like SQLite)
+		auto &storage = table.GetStorage();
+		if (storage.HasUniqueIndexes()) {
+			BindDeleteIndexColumns(table, get, del->return_columns);
+		}
 	}
 
 	del->AddChild(std::move(root));
diff --git a/src/planner/binder/statement/bind_merge_into.cpp b/src/planner/binder/statement/bind_merge_into.cpp
@@ -368,28 +368,37 @@ BoundStatement Binder::Bind(MergeIntoStatement &stmt) {
 		projection_expressions.push_back(std::move(marker_ref));
 	}
 
-	// If RETURNING is present and we have a DELETE action, add all physical columns to the scan
-	// so we can pass them through instead of fetching by row ID in PhysicalDelete.
-	// Generated columns will be computed in the RETURNING projection by the binder.
-	if (!stmt.returning_list.empty()) {
-		bool has_delete_action = false;
-		for (auto &entry : merge_into->actions) {
-			for (auto &action : entry.second) {
-				if (action->action_type == MergeActionType::MERGE_DELETE) {
-					has_delete_action = true;
-					break;
-				}
-			}
-			if (has_delete_action) {
+	// Check if we have a DELETE action
+	bool has_delete_action = false;
+	for (auto &entry : merge_into->actions) {
+		for (auto &action : entry.second) {
+			if (action->action_type == MergeActionType::MERGE_DELETE) {
+				has_delete_action = true;
 				break;
 			}
 		}
-
 		if (has_delete_action) {
+			break;
+		}
+	}
+
+	// If RETURNING is present and we have a DELETE action, add all physical columns to the scan
+	// so we can pass them through instead of fetching by row ID in PhysicalDelete.
+	// Generated columns will be computed in the RETURNING projection by the binder.
+	if (has_delete_action) {
+		if (!stmt.returning_list.empty()) {
 			// Use the overloaded helper to add physical columns to the scan and build projection expressions
 			auto &target_binding = join_ref.get().children[inverted ? 0 : 1];
 			BindDeleteReturningColumns(table, get, merge_into->delete_return_columns, projection_expressions,
 			                           *target_binding);
+		} else if (table.IsDuckTable()) {
+			// Only optimize for DuckDB tables (not attached external tables like SQLite)
+			auto &storage = table.GetStorage();
+			if (storage.HasUniqueIndexes()) {
+				auto &target_binding = join_ref.get().children[inverted ? 0 : 1];
+				BindDeleteIndexColumns(table, get, merge_into->delete_return_columns, projection_expressions,
+				                       *target_binding);
+			}
 		}
 	}
 
diff --git a/test/sql/delete/test_delete_indexed.test b/test/sql/delete/test_delete_indexed.test
diff --git a/test/sql/returning/returning_delete.test b/test/sql/returning/returning_delete.test
