diff --git a/core/translate/alter.rs b/core/translate/alter.rs index 5875b201f..77a1949ea 100644 --- a/core/translate/alter.rs +++ b/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 diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 991ec26cd..5ed3852a3 100644 --- a/core/vdbe/execute.rs +++ b/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; diff --git a/testing/alter_table.test b/testing/alter_table.test index 5a856eca9..e858d7e7b 100755 --- a/testing/alter_table.test +++ b/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; +} {{}} \ No newline at end of file diff --git a/tests/integration/fuzz/mod.rs b/tests/integration/fuzz/mod.rs index f6c713ee5..39fb60de3 100644 --- a/tests/integration/fuzz/mod.rs +++ b/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