Merge 'Fix index bookkeeping in DROP COLUMN' from Jussi Saurio

Closes #3448. Nasty bug - see issue for details

Closes #3449

core/translate/alter.rs

@@ -119,13 +119,15 @@ pub fn translate_alter_table(
 
             for index in table_indexes.iter() {
                 // Referenced in regular index
-                if index
+                let maybe_indexed_col = index
                     .columns
                     .iter()
-                    .any(|col| col.pos_in_table == dropped_index)
-                {
+                    .enumerate()
+                    .find(|(_, col)| col.pos_in_table == dropped_index);
+                if let Some((pos_in_index, indexed_col)) = maybe_indexed_col {
                     return Err(LimboError::ParseError(format!(
-                        "cannot drop column \"{column_name}\": indexed"
+                        "cannot drop column \"{column_name}\": it is referenced in the index {}; position in index is {pos_in_index}, position in table is {}",
+                        index.name, indexed_col.pos_in_table
                     )));
                 }
                 // Referenced in partial index

core/vdbe/execute.rs

@@ -7913,7 +7913,7 @@ pub fn op_drop_column(
             .get_mut(&normalized_table_name)
             .expect("table being renamed should be in schema");
 
-        let table = Arc::make_mut(table);
+        let table = Arc::get_mut(table).expect("this should be the only strong reference");
 
         let Table::BTree(btree) = table else {
             panic!("only btree tables can be renamed");
@@ -7923,29 +7923,49 @@ pub fn op_drop_column(
         btree.columns.remove(*column_index)
     });
 
-    let schema = conn.schema.read();
-    if let Some(indexes) = schema.indexes.get(&normalized_table_name) {
-        for index in indexes {
-            if index
-                .columns
-                .iter()
-                .any(|column| column.pos_in_table == *column_index)
-            {
-                return Err(LimboError::ParseError(format!(
-                    "cannot drop column \"{column_name}\": indexed"
-                )));
+    {
+        let schema = conn.schema.read();
+        if let Some(indexes) = schema.indexes.get(&normalized_table_name) {
+            for index in indexes {
+                if index
+                    .columns
+                    .iter()
+                    .any(|column| column.pos_in_table == *column_index)
+                {
+                    return Err(LimboError::ParseError(format!(
+                        "cannot drop column \"{column_name}\": indexed"
+                    )));
+                }
             }
         }
     }
 
-    for (view_name, view) in schema.views.iter() {
-        let view_select_sql = format!("SELECT * FROM {view_name}");
-        conn.prepare(view_select_sql.as_str()).map_err(|e| {
-            LimboError::ParseError(format!(
-                "cannot drop column \"{}\": referenced in VIEW {view_name}: {}",
-                column_name, view.sql,
-            ))
-        })?;
+    // Update index.pos_in_table for all indexes.
+    // For example, if the dropped column had index 2, then anything that was indexed on column 3 or higher should be decremented by 1.
+    conn.with_schema_mut(|schema| {
+        if let Some(indexes) = schema.indexes.get_mut(&normalized_table_name) {
+            for index in indexes {
+                let index = Arc::get_mut(index).expect("this should be the only strong reference");
+                for index_column in index.columns.iter_mut() {
+                    if index_column.pos_in_table > *column_index {
+                        index_column.pos_in_table -= 1;
+                    }
+                }
+            }
+        }
+    });
+
+    {
+        let schema = conn.schema.read();
+        for (view_name, view) in schema.views.iter() {
+            let view_select_sql = format!("SELECT * FROM {view_name}");
+            conn.prepare(view_select_sql.as_str()).map_err(|e| {
+                LimboError::ParseError(format!(
+                    "cannot drop column \"{}\": referenced in VIEW {view_name}: {}",
+                    column_name, view.sql,
+                ))
+            })?;
+        }
     }
 
     state.pc += 1;

testing/alter_table.test

@@ -211,3 +211,12 @@ do_execsql_test_on_specific_db {:memory:} alter-table-add-notnull-col {
 	ALTER TABLE t ADD b NOT NULL;
 	SELECT sql FROM sqlite_schema WHERE type = 'table' AND name = 't';
 } {{CREATE TABLE t (a, b NOT NULL)}}
+
+# https://github.com/tursodatabase/turso/issues/3448
+do_execsql_test_on_specific_db {:memory:} drop-column-regression {
+    CREATE TABLE t (id, col1 BLOB, col2 UNIQUE);
+    INSERT INTO t VALUES ('id', 'col1', 'col2');
+    ALTER TABLE t ADD COLUMN col3 BLOB;
+    ALTER TABLE t DROP COLUMN col1;
+    SELECT col3 FROM t;
+} {{}}

tests/integration/fuzz/mod.rs

@@ -2508,7 +2508,6 @@ mod tests {
     }
 
     #[test]
-    #[ignore]
     pub fn fuzz_long_create_table_drop_table_alter_table() {
         let db = TempDatabase::new_empty(true);
         let limbo_conn = db.connect_limbo();
@@ -2529,7 +2528,7 @@ mod tests {
 
         let mut undroppable_cols = HashSet::new();
 
-        for iteration in 0..50000 {
+        for iteration in 0..5000 {
             println!("iteration: {iteration} (seed: {seed})");
             let operation = rng.random_range(0..100); // 0: create, 1: drop, 2: alter, 3: alter rename