From 712eb823825cb0b076840c38b18858cb4e2fa8fa Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Mon, 15 Dec 2025 15:01:11 +0200 Subject: [PATCH] fix/sim: modify existing rows in ALTER COLUMN properly Bug 1: apply_snapshot(), which handles committing transaction changes to the simulator state, had this bug: let's say a transaction does: 1. ALTER TABLE t ADD COLUMN (e.g., adding 9th column) 2. INSERT INTO t ... (populating all 9 columns) then apply_snapshot() would: 1. First push the INSERT row (with 9 values) to committed_tables 2. Then detect new_col_count (9) > old_col_count (8) and add NULLs to ALL rows hence, adding a 10th column to the new row too. Fix 1: only add nulls to existing rows that weren't inserted in the same tx after the ADD COLUMN. Bug 2: the exact same thing, but for DROP COLUMN. Fix 2: only remove the i'th column from rows that existed prior to the DROP COLUMN was issued. --- simulator/runner/env.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/simulator/runner/env.rs b/simulator/runner/env.rs index 2e3a0892d..f7bd52a67 100644 --- a/simulator/runner/env.rs +++ b/simulator/runner/env.rs @@ -226,10 +226,11 @@ where let new_col_count = current_table.columns.len(); if new_col_count > old_col_count { - // ADD COLUMN: fill with NULL - let nulls_to_add = new_col_count - old_col_count; + // ADD COLUMN: fill with NULL only for rows that need it. + // Rows inserted after ADD COLUMN in the same transaction + // already have the correct number of values. for row in &mut committed.rows { - for _ in 0..nulls_to_add { + while row.len() < new_col_count { row.push(SimValue::NULL); } } @@ -250,8 +251,13 @@ where dropped_indices.sort_by(|a, b| b.cmp(a)); for row in &mut committed.rows { - for &idx in &dropped_indices { - row.remove(idx); + // Only remove from rows that have the old column count. + // Rows inserted after DROP COLUMN in the same transaction + // already have the correct (new) number of values. + if row.len() == old_col_count { + for &idx in &dropped_indices { + row.remove(idx); + } } } }