Align trigger sql rewrite behavior with sqlite

SQLite doesn't rewrite INSERT lists or WHEN clause, it instead
lets the trigger go "stale" and will cause runtime errors. This
may not be great behavior, but it's compatible...

core/translate/alter.rs

@@ -620,6 +620,68 @@ pub fn translate_alter_table(
                 let target_table_name_norm = normalize_ident(table_name);
                 for trigger in resolver.schema.triggers.values().flatten() {
                     let trigger_table_name_norm = normalize_ident(&trigger.table_name);
+
+                    // SQLite fails RENAME COLUMN if a trigger's WHEN clause references the column
+                    // Check for WHEN clause references first
+                    if trigger_table_name_norm == target_table_name_norm {
+                        if let Some(ref when_expr) = trigger.when_clause {
+                            let column_name_norm = normalize_ident(from);
+                            let trigger_table = resolver
+                                .schema
+                                .get_btree_table(&trigger_table_name_norm)
+                                .ok_or_else(|| {
+                                    LimboError::ParseError(format!(
+                                        "trigger table not found: {trigger_table_name_norm}"
+                                    ))
+                                })?;
+
+                            let mut found_in_when = false;
+                            use crate::translate::expr::walk_expr;
+                            walk_expr(when_expr, &mut |e: &ast::Expr| -> Result<
+                                crate::translate::expr::WalkControl,
+                            > {
+                                match e {
+                                    ast::Expr::Qualified(ns, col)
+                                    | ast::Expr::DoublyQualified(_, ns, col) => {
+                                        let ns_norm = normalize_ident(ns.as_str());
+                                        let col_norm = normalize_ident(col.as_str());
+
+                                        // Check NEW.column or OLD.column
+                                        if (ns_norm.eq_ignore_ascii_case("new")
+                                            || ns_norm.eq_ignore_ascii_case("old"))
+                                            && col_norm == column_name_norm
+                                        {
+                                            found_in_when = true;
+                                            return Ok(
+                                                crate::translate::expr::WalkControl::SkipChildren,
+                                            );
+                                        }
+                                    }
+                                    ast::Expr::Id(col) => {
+                                        // Unqualified column reference - check if it matches
+                                        let col_norm = normalize_ident(col.as_str());
+                                        if col_norm == column_name_norm {
+                                            // Verify this column exists in the trigger's owning table
+                                            if trigger_table.get_column(&col_norm).is_some() {
+                                                found_in_when = true;
+                                                return Ok(crate::translate::expr::WalkControl::SkipChildren);
+                                            }
+                                        }
+                                    }
+                                    _ => {}
+                                }
+                                Ok(crate::translate::expr::WalkControl::Continue)
+                            })?;
+
+                            if found_in_when {
+                                return Err(LimboError::ParseError(format!(
+                                    "error in trigger {}: no such column: {}",
+                                    trigger.name, from
+                                )));
+                            }
+                        }
+                    }
+
                     let mut needs_rewrite = false;
 
                     // Check if trigger references the column being renamed
@@ -948,38 +1010,38 @@ fn trigger_references_column(
     }
 
     // Check all trigger commands
+    // Note: We only check NEW.x, OLD.x, and unqualified x references in expressions.
+    // INSERT column lists and UPDATE SET column lists are NOT checked here because
+    // SQLite allows DROP COLUMN even when triggers reference columns in INSERT/UPDATE
+    // column lists targeting the owning table. The error only occurs when the trigger
+    // is actually executed.
     for cmd in &trigger.commands {
         match cmd {
             ast::TriggerCmd::Update {
                 sets, where_clause, ..
             } => {
-                // Check SET assignments
+                // Check SET expressions (not column names in SET clause)
                 for set in sets {
                     walk_expr(&set.expr, &mut |e: &ast::Expr| -> Result<WalkControl> {
                         check_column_ref(e, table, &column_name_norm, &mut found)?;
                         Ok(WalkControl::Continue)
                     })?;
-                }
-                // Check WHERE clause
-                if let Some(ref where_expr) = where_clause {
-                    walk_expr(where_expr, &mut |e: &ast::Expr| -> Result<WalkControl> {
-                        check_column_ref(e, table, &column_name_norm, &mut found)?;
-                        Ok(WalkControl::Continue)
-                    })?;
-                }
-            }
-            ast::TriggerCmd::Insert {
-                select, col_names, ..
-            } => {
-                // Check column names in INSERT
-                for col_name in col_names {
-                    let col_norm = normalize_ident(col_name.as_str());
-                    if col_norm == column_name_norm {
-                        found = true;
+                    if found {
                         break;
                     }
                 }
-                // Check SELECT/VALUES expressions
+                // Check WHERE clause
+                if !found {
+                    if let Some(ref where_expr) = where_clause {
+                        walk_expr(where_expr, &mut |e: &ast::Expr| -> Result<WalkControl> {
+                            check_column_ref(e, table, &column_name_norm, &mut found)?;
+                            Ok(WalkControl::Continue)
+                        })?;
+                    }
+                }
+            }
+            ast::TriggerCmd::Insert { select, .. } => {
+                // Check SELECT/VALUES expressions (not column names in INSERT clause)
                 walk_select_expressions(select, table, &column_name_norm, &mut found)?;
             }
             ast::TriggerCmd::Delete { where_clause, .. } => {
@@ -1434,22 +1496,10 @@ fn rewrite_trigger_sql_for_column_rename(
         event
     };
 
-    // Clone and rewrite expressions
-    let new_when_clause = when_clause
-        .as_ref()
-        .map(|e| {
-            rewrite_expr_for_column_rename(
-                e,
-                &trigger_table,
-                &trigger_table_name,
-                &target_table_name,
-                &old_col_norm,
-                &new_col_norm,
-                None, // WHEN clause: unqualified refs refer to trigger's owning table
-                resolver,
-            )
-        })
-        .transpose()?;
+    // Note: SQLite fails RENAME COLUMN if a trigger's WHEN clause references the column.
+    // We check for this earlier and fail the operation immediately, matching SQLite.
+    // If we reach here, the WHEN clause doesn't reference the column, so we keep it unchanged.
+    let new_when_clause = when_clause.clone();
 
     let mut new_commands = Vec::new();
     for cmd in commands {
@@ -1476,7 +1526,7 @@ fn rewrite_trigger_sql_for_column_rename(
         &new_event,
         &tbl_name,
         for_each_row,
-        new_when_clause.as_ref(),
+        new_when_clause.as_deref(),
         &new_commands,
     );
 

tests/integration/trigger.rs

@@ -1229,7 +1229,7 @@ fn test_alter_table_rename_column_propagates_to_trigger_with_multiple_references
 }
 
 #[test]
-fn test_alter_table_rename_column_propagates_to_trigger_when_clause() {
+fn test_alter_table_rename_column_fails_when_trigger_when_clause_references_column() {
     let db = TempDatabase::new_empty();
     let conn = db.connect_limbo();
 
@@ -1245,34 +1245,17 @@ fn test_alter_table_rename_column_propagates_to_trigger_when_clause() {
     )
     .unwrap();
 
-    // Rename column y to y_new
-    conn.execute("ALTER TABLE t RENAME COLUMN y TO y_new")
-        .unwrap();
+    // Rename column y to y_new should fail (SQLite fails if WHEN clause references the column)
+    let result = conn.execute("ALTER TABLE t RENAME COLUMN y TO y_new");
+    assert!(
+        result.is_err(),
+        "RENAME COLUMN should fail when trigger WHEN clause references the column"
+    );
 
-    // Verify trigger SQL was updated
-    let mut stmt = conn
-        .prepare("SELECT sql FROM sqlite_schema WHERE type='trigger' AND name='tu'")
-        .unwrap();
-    let mut trigger_sql = String::new();
-    loop {
-        match stmt.step().unwrap() {
-            turso_core::StepResult::Row => {
-                let row = stmt.row().unwrap();
-                trigger_sql = row.get_value(0).cast_text().unwrap().to_string();
-            }
-            turso_core::StepResult::Done => break,
-            turso_core::StepResult::IO => {
-                stmt.run_once().unwrap();
-            }
-            _ => panic!("Unexpected step result"),
-        }
-    }
-
-    // Trigger SQL WHEN clause should reference y_new
-    let normalized_sql = trigger_sql.split_whitespace().collect::<Vec<_>>().join(" ");
-    assert_eq!(
-        normalized_sql,
-        "CREATE TRIGGER tu BEFORE INSERT ON t WHEN y_new > 10 BEGIN INSERT INTO t VALUES (NEW.x, 100); END"
+    let error_msg = result.unwrap_err().to_string();
+    assert!(
+        error_msg.contains("error in trigger") && error_msg.contains("no such column"),
+        "Error should mention trigger and column: {error_msg}",
     );
 }
 
@@ -1830,3 +1813,107 @@ fn test_alter_table_rename_column_update_of_multiple_columns_rewritten() {
         "Trigger SQL should NOT contain UPDATE OF x (x was renamed to x_new): {normalized_sql}",
     );
 }
+
+#[test]
+fn test_alter_table_drop_column_allows_when_insert_targets_other_table() {
+    let db = TempDatabase::new_empty();
+    let conn = db.connect_limbo();
+
+    // Create two tables, both with column 'x'
+    conn.execute("CREATE TABLE t (x INTEGER, y INTEGER)")
+        .unwrap();
+    conn.execute("CREATE TABLE u (x INTEGER, z INTEGER)")
+        .unwrap();
+
+    // Create trigger on t that inserts into u(x) - this should NOT prevent dropping x from t
+    conn.execute(
+        "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN
+         INSERT INTO u(x) VALUES (NEW.y);
+        END",
+    )
+    .unwrap();
+
+    // Dropping column x from table t should succeed (INSERT INTO u(x) refers to u.x, not t.x)
+    conn.execute("ALTER TABLE t DROP COLUMN x").unwrap();
+}
+
+#[test]
+fn test_alter_table_drop_column_allows_when_update_targets_other_table() {
+    let db = TempDatabase::new_empty();
+    let conn = db.connect_limbo();
+
+    // Create two tables, both with column 'x'
+    conn.execute("CREATE TABLE t (x INTEGER, y INTEGER)")
+        .unwrap();
+    conn.execute("CREATE TABLE u (x INTEGER, z INTEGER)")
+        .unwrap();
+
+    // Create trigger on t that updates u SET x = ... - this should NOT prevent dropping x from t
+    conn.execute(
+        "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN
+         UPDATE u SET x = NEW.y WHERE z = 1;
+        END",
+    )
+    .unwrap();
+
+    // Dropping column x from table t should succeed (UPDATE u SET x refers to u.x, not t.x)
+    conn.execute("ALTER TABLE t DROP COLUMN x").unwrap();
+}
+
+#[test]
+fn test_alter_table_drop_column_allows_when_insert_targets_owning_table() {
+    let db = TempDatabase::new_empty();
+    let conn = db.connect_limbo();
+
+    // Create table
+    conn.execute("CREATE TABLE t (x INTEGER, y INTEGER)")
+        .unwrap();
+
+    // Create trigger on t that inserts into t(x) - SQLite allows DROP COLUMN here
+    // The error only occurs when the trigger is actually executed
+    conn.execute(
+        "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN
+         INSERT INTO t(x) VALUES (NEW.y);
+        END",
+    )
+    .unwrap();
+
+    // Dropping column x from table t should succeed (SQLite allows this)
+    conn.execute("ALTER TABLE t DROP COLUMN x").unwrap();
+
+    // Verify that executing the trigger now causes an error
+    let result = conn.execute("INSERT INTO t VALUES (5)");
+    assert!(
+        result.is_err(),
+        "INSERT should fail because trigger references dropped column"
+    );
+}
+
+#[test]
+fn test_alter_table_drop_column_allows_when_update_set_targets_owning_table() {
+    let db = TempDatabase::new_empty();
+    let conn = db.connect_limbo();
+
+    // Create table
+    conn.execute("CREATE TABLE t (x INTEGER, y INTEGER)")
+        .unwrap();
+
+    // Create trigger on t that updates t SET x = ... - SQLite allows DROP COLUMN here
+    // The error only occurs when the trigger is actually executed
+    conn.execute(
+        "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN
+         UPDATE t SET x = NEW.y WHERE y = 1;
+        END",
+    )
+    .unwrap();
+
+    // Dropping column x from table t should succeed (SQLite allows this)
+    conn.execute("ALTER TABLE t DROP COLUMN x").unwrap();
+
+    // Verify that executing the trigger now causes an error
+    let result = conn.execute("INSERT INTO t VALUES (5)");
+    assert!(
+        result.is_err(),
+        "INSERT should fail because trigger references dropped column"
+    );
+}