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