From 4e4e6f212199d88f379ae07d36ea97057c4bb215 Mon Sep 17 00:00:00 2001 From: Leonid Krugliak Date: Mon, 19 Jan 2026 08:35:56 +0200 Subject: [PATCH] Support generated columns in DELETE RETURNING optimization Previously, DELETE RETURNING would fall back to fetching columns by row ID when the table had generated columns. This was because generated columns cannot be scanned directly - they must be computed from their expressions. This change allows the optimization to work with generated columns by: 1. Scanning all physical columns and passing them through from the scan 2. Letting the RETURNING projection compute generated columns using their expressions (same approach as INSERT/UPDATE RETURNING) The comment in the fallback path is updated to reflect that generated columns are no longer a reason to use the fallback path. --- .../delete_returning_3col_generated.benchmark | 2 +- .../operator/persistent/physical_delete.cpp | 2 +- src/planner/binder/statement/bind_delete.cpp | 41 +++++++++++-------- test/sql/returning/returning_delete.test | 3 +- 4 files changed, 29 insertions(+), 19 deletions(-) diff --git a/benchmark/micro/delete_returning/delete_returning_3col_generated.benchmark b/benchmark/micro/delete_returning/delete_returning_3col_generated.benchmark index c69d60a84565..3f27972be053 100644 --- a/benchmark/micro/delete_returning/delete_returning_3col_generated.benchmark +++ b/benchmark/micro/delete_returning/delete_returning_3col_generated.benchmark @@ -1,5 +1,5 @@ # name: benchmark/micro/delete_returning/delete_returning_3col_generated.benchmark -# description: DELETE RETURNING with 3 columns including generated (fallback fetch-by-rowid path) +# description: DELETE RETURNING with 3 columns including generated column # group: [delete_returning] load diff --git a/src/execution/operator/persistent/physical_delete.cpp b/src/execution/operator/persistent/physical_delete.cpp index 8aefe29c26a1..4e01e5c8469a 100644 --- a/src/execution/operator/persistent/physical_delete.cpp +++ b/src/execution/operator/persistent/physical_delete.cpp @@ -83,6 +83,7 @@ SinkResultType PhysicalDelete::Sink(ExecutionContext &context, DataChunk &chunk, 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); l_state.delete_chunk.data[i].Reference(chunk.data[return_columns[i]]); @@ -91,7 +92,6 @@ SinkResultType PhysicalDelete::Sink(ExecutionContext &context, DataChunk &chunk, } else { // Fall back to fetching columns by row ID // This path is used when: - // - Table has generated columns (can't be scanned, must be computed) // - Unique indexes exist but no RETURNING (need indexed columns for delete tracking) // - MERGE INTO operations (optimization not implemented there yet) auto &transaction = DuckTransaction::Get(context.client, table.db); diff --git a/src/planner/binder/statement/bind_delete.cpp b/src/planner/binder/statement/bind_delete.cpp index 6d00816d2a52..e38cb76699f2 100644 --- a/src/planner/binder/statement/bind_delete.cpp +++ b/src/planner/binder/statement/bind_delete.cpp @@ -64,29 +64,38 @@ BoundStatement Binder::Bind(DeleteStatement &stmt) { auto del = make_uniq(table, GenerateTableIndex()); del->bound_constraints = BindConstraints(table); - // If RETURNING is present, add all table columns to the scan so we can pass them through - // instead of having to fetch them by row ID in PhysicalDelete - // Skip this optimization if the table has generated columns, as they need to be computed - // rather than scanned - if (!stmt.returning_list.empty() && !table.HasGeneratedColumns()) { + // 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. + if (!stmt.returning_list.empty()) { auto &column_ids = get.GetColumnIds(); - auto column_count = table.GetColumns().LogicalColumnCount(); + auto &columns = table.GetColumns(); + auto physical_count = columns.PhysicalColumnCount(); + + // Build a map from storage index -> input chunk index + // return_columns[storage_idx] = input_chunk_idx + del->return_columns.resize(physical_count, DConstants::INVALID_INDEX); - // Build a map of which table columns are already in the scan - // and track their indices in the input chunk - del->return_columns.resize(column_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() && col_id.GetPrimaryIndex() < column_count) { - del->return_columns[col_id.GetPrimaryIndex()] = chunk_idx; + if (col_id.IsVirtualColumn()) { + continue; + } + // Get the column by logical index, then get its storage index + auto logical_idx = col_id.GetPrimaryIndex(); + if (!columns.GetColumn(LogicalIndex(logical_idx)).Generated()) { + auto storage_idx = columns.GetColumn(LogicalIndex(logical_idx)).StorageOid(); + del->return_columns[storage_idx] = chunk_idx; } } - // Add any missing columns to the scan - for (idx_t col_idx = 0; col_idx < column_count; col_idx++) { - if (del->return_columns[col_idx] == DConstants::INVALID_INDEX) { - del->return_columns[col_idx] = column_ids.size(); - get.AddColumnId(col_idx); + // Add any missing physical columns to the scan + for (auto &col : columns.Physical()) { + auto storage_idx = col.StorageOid(); + if (del->return_columns[storage_idx] == DConstants::INVALID_INDEX) { + del->return_columns[storage_idx] = column_ids.size(); + get.AddColumnId(col.Logical().index); } } } diff --git a/test/sql/returning/returning_delete.test b/test/sql/returning/returning_delete.test index 2222d3823751..cc392106ca52 100644 --- a/test/sql/returning/returning_delete.test +++ b/test/sql/returning/returning_delete.test @@ -323,7 +323,8 @@ statement ok DROP TABLE test_where_returning; # ============================================================ -# Tests for DELETE RETURNING with generated columns (fallback path) +# Tests for DELETE RETURNING with generated columns +# Generated columns are computed at execution time from physical columns # ============================================================ # Test DELETE RETURNING with VIRTUAL generated column