diff --git a/core/translate/alter.rs b/core/translate/alter.rs index 2505bc273..95aecc3d5 100644 --- a/core/translate/alter.rs +++ b/core/translate/alter.rs @@ -166,7 +166,11 @@ pub fn translate_alter_table( ); let where_copy = index .bind_where_expr(Some(&mut table_references), connection) - .expect("where clause to exist"); + .ok_or_else(|| { + LimboError::ParseError( + "index where clause unexpectedly missing".to_string(), + ) + })?; let mut column_referenced = false; walk_expr( &where_copy, @@ -201,6 +205,16 @@ pub fn translate_alter_table( // References in VIEWs are checked in the VDBE layer op_drop_column instruction. + // Check if any trigger owned by this table references the column being dropped + for trigger in resolver.schema.get_triggers_for_table(table_name) { + if trigger_references_column(trigger, &btree, column_name)? { + return Err(LimboError::ParseError(format!( + "cannot drop column \"{column_name}\": it is referenced in trigger {}", + trigger.name + ))); + } + } + btree.columns.remove(dropped_index); let sql = btree.to_sql().replace('\'', "''"); @@ -214,8 +228,13 @@ pub fn translate_alter_table( ); let mut parser = Parser::new(stmt.as_bytes()); - let Some(ast::Cmd::Stmt(ast::Stmt::Update(update))) = parser.next_cmd().unwrap() else { - unreachable!(); + let cmd = parser.next_cmd().map_err(|e| { + LimboError::ParseError(format!("failed to parse generated UPDATE statement: {e}")) + })?; + let Some(ast::Cmd::Stmt(ast::Stmt::Update(update))) = cmd else { + return Err(LimboError::ParseError( + "generated UPDATE statement did not parse as expected".to_string(), + )); }; translate_update_for_schema_change( @@ -322,7 +341,11 @@ pub fn translate_alter_table( } } - let new_column_name = column.name.as_ref().unwrap(); + let new_column_name = column.name.as_ref().ok_or_else(|| { + LimboError::ParseError( + "column name is missing in ALTER TABLE ADD COLUMN".to_string(), + ) + })?; if btree.get_column(new_column_name).is_some() { return Err(LimboError::ParseError( "duplicate column name: ".to_string() + new_column_name, @@ -403,8 +426,13 @@ pub fn translate_alter_table( ); let mut parser = Parser::new(stmt.as_bytes()); - let Some(ast::Cmd::Stmt(ast::Stmt::Update(update))) = parser.next_cmd().unwrap() else { - unreachable!(); + let cmd = parser.next_cmd().map_err(|e| { + LimboError::ParseError(format!("failed to parse generated UPDATE statement: {e}")) + })?; + let Some(ast::Cmd::Stmt(ast::Stmt::Update(update))) = cmd else { + return Err(LimboError::ParseError( + "generated UPDATE statement did not parse as expected".to_string(), + )); }; translate_update_for_schema_change( @@ -443,10 +471,15 @@ pub fn translate_alter_table( ))); }; - let sqlite_schema = resolver - .schema - .get_btree_table(SQLITE_TABLEID) - .expect("sqlite_schema should be on schema"); + let sqlite_schema = + resolver + .schema + .get_btree_table(SQLITE_TABLEID) + .ok_or_else(|| { + LimboError::ParseError( + "sqlite_schema table not found in schema".to_string(), + ) + })?; let cursor_id = program.alloc_cursor_id(CursorType::BTreeTable(sqlite_schema.clone())); @@ -579,10 +612,121 @@ pub fn translate_alter_table( )); } - let sqlite_schema = resolver - .schema - .get_btree_table(SQLITE_TABLEID) - .expect("sqlite_schema should be on schema"); + // If renaming, rewrite trigger SQL for all triggers that reference this column + // We'll collect the triggers to rewrite and update them in sqlite_schema + let mut triggers_to_rewrite: Vec<(String, String)> = Vec::new(); + if rename { + // Find all triggers that might reference this column + 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 + // or if trigger commands contain qualified references to the trigger table (e.g., t.x) + if trigger_table_name_norm == target_table_name_norm { + 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}" + )) + })?; + + // Check WHEN clause + if let Some(ref when_expr) = trigger.when_clause { + if expr_references_trigger_column( + when_expr, + &trigger_table, + &trigger_table_name_norm, + &column_name_norm, + )? { + return Err(LimboError::ParseError(format!( + "error in trigger {}: no such column: {}", + trigger.name, from + ))); + } + } + + // Check trigger commands for qualified references to trigger table (e.g., t.x) + // SQLite fails RENAME COLUMN if triggers contain qualified references like t.x + if check_trigger_has_qualified_ref_to_column( + trigger, + &trigger_table, + &trigger_table_name_norm, + &column_name_norm, + )? { + return Err(LimboError::ParseError(format!( + "error in trigger {}: no such column: {}", + trigger.name, from + ))); + } + } + + // Check if trigger references the column being renamed + // This includes: + // 1. References from the trigger's owning table (NEW.x, OLD.x, unqualified x) + // 2. References in INSERT column lists targeting the table being renamed + // 3. References in UPDATE SET column lists targeting the table being renamed + let mut needs_rewrite = false; + + if trigger_table_name_norm == target_table_name_norm { + // Trigger is on the table being renamed - check for references + 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}" + )) + })?; + + needs_rewrite = trigger_references_column(trigger, &trigger_table, from)?; + } + + // Also check if trigger references the column in INSERT/UPDATE targeting other tables + // Parse the trigger to check INSERT column lists and UPDATE SET column lists + if !needs_rewrite { + needs_rewrite = trigger_references_column_in_other_tables( + trigger, + &target_table_name_norm, + from, + )?; + } + + if needs_rewrite { + match rewrite_trigger_sql_for_column_rename( + &trigger.sql, + table_name, + from, + col_name, + resolver, + ) { + Ok(new_sql) => { + triggers_to_rewrite.push((trigger.name.clone(), new_sql)); + } + Err(e) => { + // If we can't rewrite the trigger, fail the ALTER TABLE operation + return Err(LimboError::ParseError(format!( + "error in trigger {} after rename column: {}", + trigger.name, e + ))); + } + } + } + } + } + + let sqlite_schema = + resolver + .schema + .get_btree_table(SQLITE_TABLEID) + .ok_or_else(|| { + LimboError::ParseError( + "sqlite_schema table not found in schema".to_string(), + ) + })?; let cursor_id = program.alloc_cursor_id(CursorType::BTreeTable(sqlite_schema.clone())); @@ -646,6 +790,39 @@ pub fn translate_alter_table( }); }); + // Update trigger SQL for renamed columns + for (trigger_name, new_sql) in triggers_to_rewrite { + let escaped_sql = new_sql.replace('\'', "''"); + let update_stmt = format!( + r#" + UPDATE {SQLITE_TABLEID} + SET sql = '{escaped_sql}' + WHERE name = '{trigger_name}' COLLATE NOCASE AND type = 'trigger' + "#, + ); + + let mut parser = Parser::new(update_stmt.as_bytes()); + let cmd = parser.next_cmd().map_err(|e| { + LimboError::ParseError(format!( + "failed to parse trigger update SQL for {trigger_name}: {e}" + )) + })?; + let Some(ast::Cmd::Stmt(ast::Stmt::Update(update))) = cmd else { + return Err(LimboError::ParseError(format!( + "failed to parse trigger update SQL for {trigger_name}", + ))); + }; + + program = translate_update_for_schema_change( + update, + resolver, + program, + connection, + input, + |_program| {}, + )?; + } + program.emit_insn(Insn::SetCookie { db: 0, cookie: Cookie::SchemaVersion, @@ -686,7 +863,9 @@ fn translate_rename_virtual_table( let sqlite_schema = resolver .schema .get_btree_table(SQLITE_TABLEID) - .expect("sqlite_schema should be on schema"); + .ok_or_else(|| { + LimboError::ParseError("sqlite_schema table not found in schema".to_string()) + })?; let schema_cur = program.alloc_cursor_id(CursorType::BTreeTable(sqlite_schema.clone())); program.emit_insn(Insn::OpenWrite { @@ -762,3 +941,1580 @@ fn translate_rename_virtual_table( Ok(program) } + +/* Triggers must be rewritten when a column is renamed, and DROP COLUMN on table T must be forbidden if any trigger on T references the column. +Here are some helpers related to that: */ + +/// Check if a trigger contains qualified references to a specific column in its owning table. +/// This is used to detect cases like `t.x` in a trigger on table `t`, which SQLite fails on RENAME COLUMN. +/// Only checks for qualified table references (e.g., t.x), not NEW.x, OLD.x, or unqualified x. +fn check_trigger_has_qualified_ref_to_column( + trigger: &crate::schema::Trigger, + trigger_table: &crate::schema::BTreeTable, + trigger_table_name_norm: &str, + column_name_norm: &str, +) -> Result { + use crate::translate::expr::walk_expr; + use std::cell::Cell; + + let found = Cell::new(false); + + // Helper callback to check for qualified references to trigger table + let mut check_qualified_ref = |e: &ast::Expr| -> Result { + if let ast::Expr::Qualified(ns, col) | ast::Expr::DoublyQualified(_, ns, col) = e { + let ns_norm = normalize_ident(ns.as_str()); + let col_norm = normalize_ident(col.as_str()); + // Only check for qualified refs to trigger table (not NEW/OLD) + if !ns_norm.eq_ignore_ascii_case("new") + && !ns_norm.eq_ignore_ascii_case("old") + && ns_norm == trigger_table_name_norm + && col_norm == column_name_norm + && trigger_table.get_column(&col_norm).is_some() + { + found.set(true); + return Ok(crate::translate::expr::WalkControl::SkipChildren); + } + } + Ok(crate::translate::expr::WalkControl::Continue) + }; + + for cmd in &trigger.commands { + match cmd { + ast::TriggerCmd::Update { + sets, where_clause, .. + } => { + for set in sets { + walk_expr(&set.expr, &mut check_qualified_ref)?; + if found.get() { + return Ok(true); + } + } + if let Some(ref where_expr) = where_clause { + walk_expr(where_expr, &mut check_qualified_ref)?; + if found.get() { + return Ok(true); + } + } + } + ast::TriggerCmd::Insert { select, .. } | ast::TriggerCmd::Select(select) => { + // Walk through all expressions in SELECT checking for qualified refs + if let Some(ref with_clause) = select.with { + for cte in &with_clause.ctes { + if walk_one_select_expressions_for_qualified_ref( + &cte.select.body.select, + &mut check_qualified_ref, + )? { + return Ok(true); + } + } + } + if walk_one_select_expressions_for_qualified_ref( + &select.body.select, + &mut check_qualified_ref, + )? { + return Ok(true); + } + for compound in &select.body.compounds { + if walk_one_select_expressions_for_qualified_ref( + &compound.select, + &mut check_qualified_ref, + )? { + return Ok(true); + } + } + for sorted_col in &select.order_by { + walk_expr(&sorted_col.expr, &mut check_qualified_ref)?; + if found.get() { + return Ok(true); + } + } + if let Some(ref limit) = select.limit { + walk_expr(&limit.expr, &mut check_qualified_ref)?; + if found.get() { + return Ok(true); + } + if let Some(ref offset) = limit.offset { + walk_expr(offset, &mut check_qualified_ref)?; + if found.get() { + return Ok(true); + } + } + } + } + ast::TriggerCmd::Delete { where_clause, .. } => { + if let Some(ref where_expr) = where_clause { + walk_expr(where_expr, &mut check_qualified_ref)?; + if found.get() { + return Ok(true); + } + } + } + } + } + + Ok(false) +} + +/// Helper to walk OneSelect expressions checking for qualified references +/// Uses a callback that can mutate the found flag through a closure +fn walk_one_select_expressions_for_qualified_ref( + one_select: &ast::OneSelect, + check_fn: &mut F, +) -> Result +where + F: FnMut(&ast::Expr) -> Result, +{ + use crate::translate::expr::walk_expr; + + match one_select { + ast::OneSelect::Select { + columns, + from, + where_clause, + group_by, + window_clause, + .. + } => { + for col in columns { + if let ast::ResultColumn::Expr(expr, _) = col { + if matches!( + walk_expr(expr, check_fn)?, + crate::translate::expr::WalkControl::SkipChildren + ) { + return Ok(true); + } + } + } + if let Some(ref from_clause) = from { + // Walk FROM clause expressions manually using walk_expr + match from_clause.select.as_ref() { + ast::SelectTable::Select(select, _) => { + // Recursively check the subquery + if walk_one_select_expressions_for_qualified_ref( + &select.body.select, + check_fn, + )? { + return Ok(true); + } + } + ast::SelectTable::TableCall(_, args, _) => { + for arg in args { + if matches!( + walk_expr(arg, check_fn)?, + crate::translate::expr::WalkControl::SkipChildren + ) { + return Ok(true); + } + } + } + _ => {} + } + for join in &from_clause.joins { + if let Some(ast::JoinConstraint::On(ref expr)) = join.constraint { + let skip_children = matches!( + walk_expr(expr, check_fn)?, + crate::translate::expr::WalkControl::SkipChildren + ); + if skip_children { + return Ok(true); + } + } + } + } + if let Some(ref where_expr) = where_clause { + if matches!( + walk_expr(where_expr, check_fn)?, + crate::translate::expr::WalkControl::SkipChildren + ) { + return Ok(true); + } + } + if let Some(ref group_by) = group_by { + for expr in &group_by.exprs { + if matches!( + walk_expr(expr, check_fn)?, + crate::translate::expr::WalkControl::SkipChildren + ) { + return Ok(true); + } + } + if let Some(ref having_expr) = group_by.having { + if matches!( + walk_expr(having_expr, check_fn)?, + crate::translate::expr::WalkControl::SkipChildren + ) { + return Ok(true); + } + } + } + for window_def in window_clause { + for expr in &window_def.window.partition_by { + if matches!( + walk_expr(expr, check_fn)?, + crate::translate::expr::WalkControl::SkipChildren + ) { + return Ok(true); + } + } + for sorted_col in &window_def.window.order_by { + if matches!( + walk_expr(&sorted_col.expr, check_fn)?, + crate::translate::expr::WalkControl::SkipChildren + ) { + return Ok(true); + } + } + } + } + ast::OneSelect::Values(values) => { + for row in values { + for expr in row { + if matches!( + walk_expr(expr, check_fn)?, + crate::translate::expr::WalkControl::SkipChildren + ) { + return Ok(true); + } + } + } + } + } + Ok(false) +} + +/// Check if an expression references a specific column from a trigger's owning table. +/// Checks for NEW.column, OLD.column, unqualified column references, and qualified +/// references to the trigger table (e.g., t.x in a trigger on table t). +/// Returns true if the column is found, false otherwise. +fn expr_references_trigger_column( + expr: &ast::Expr, + table: &crate::schema::BTreeTable, + trigger_table_name: &str, + column_name_norm: &str, +) -> Result { + use crate::translate::expr::walk_expr; + let trigger_table_name_norm = normalize_ident(trigger_table_name); + let mut found = false; + walk_expr(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 = true; + return Ok(crate::translate::expr::WalkControl::SkipChildren); + } + // Check qualified reference to trigger table (e.g., t.x) + if ns_norm == trigger_table_name_norm + && col_norm == *column_name_norm + && table.get_column(&col_norm).is_some() + { + found = 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 table.get_column(&col_norm).is_some() { + found = true; + return Ok(crate::translate::expr::WalkControl::SkipChildren); + } + } + } + _ => {} + } + Ok(crate::translate::expr::WalkControl::Continue) + })?; + Ok(found) +} + +/// Check if a trigger references a specific column from its owning table. +/// Returns true if the column is referenced as old.x, new.x, or unqualified x. +fn trigger_references_column( + trigger: &crate::schema::Trigger, + table: &crate::schema::BTreeTable, + column_name: &str, +) -> Result { + let column_name_norm = normalize_ident(column_name); + let mut found = false; + + // Check when_clause + if let Some(ref when_expr) = trigger.when_clause { + // Get trigger table name for checking qualified references + let trigger_table_name = normalize_ident(&trigger.table_name); + found = expr_references_trigger_column( + when_expr, + table, + &trigger_table_name, + &column_name_norm, + )?; + } + + if found { + return Ok(true); + } + + // 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 expressions (not column names in SET clause) + let trigger_table_name = normalize_ident(&trigger.table_name); + for set in sets { + if expr_references_trigger_column( + &set.expr, + table, + &trigger_table_name, + &column_name_norm, + )? { + found = true; + break; + } + } + // Check WHERE clause + if !found { + if let Some(ref where_expr) = where_clause { + found = expr_references_trigger_column( + where_expr, + table, + &trigger_table_name, + &column_name_norm, + )?; + } + } + } + ast::TriggerCmd::Insert { select, .. } => { + // Check SELECT/VALUES expressions (not column names in INSERT clause) + let trigger_table_name = normalize_ident(&trigger.table_name); + walk_select_expressions( + select, + table, + &trigger_table_name, + &column_name_norm, + &mut found, + )?; + } + ast::TriggerCmd::Delete { where_clause, .. } => { + if let Some(ref where_expr) = where_clause { + let trigger_table_name = normalize_ident(&trigger.table_name); + found = expr_references_trigger_column( + where_expr, + table, + &trigger_table_name, + &column_name_norm, + )?; + } + } + ast::TriggerCmd::Select(select) => { + let trigger_table_name = normalize_ident(&trigger.table_name); + walk_select_expressions( + select, + table, + &trigger_table_name, + &column_name_norm, + &mut found, + )?; + } + } + if found { + break; + } + } + + Ok(found) +} + +/// Helper to check if an expression references a column. +/// Used as a callback within walk_expr, so it mutates a found flag. +/// Also checks for qualified references to the trigger table (e.g., t.x). +fn check_column_ref( + e: &ast::Expr, + table: &crate::schema::BTreeTable, + trigger_table_name: &str, + column_name_norm: &str, + found: &mut bool, +) -> Result<()> { + let trigger_table_name_norm = normalize_ident(trigger_table_name); + 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()); + + let new_or_old = (ns_norm.eq_ignore_ascii_case("new") + || ns_norm.eq_ignore_ascii_case("old")) + && col_norm == *column_name_norm; + let qualified_ref = ns_norm == trigger_table_name_norm + && col_norm == *column_name_norm + && table.get_column(&col_norm).is_some(); + if new_or_old || qualified_ref { + *found = true; + } + } + 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 table.get_column(&col_norm).is_some() { + *found = true; + } + } + } + _ => {} + } + Ok(()) +} + +/// Check if a trigger references a column from a table other than its owning table. +/// This checks INSERT column lists and UPDATE SET column lists that target the table being renamed. +fn trigger_references_column_in_other_tables( + trigger: &crate::schema::Trigger, + target_table_name: &str, + column_name: &str, +) -> Result { + let column_name_norm = normalize_ident(column_name); + let target_table_name_norm = normalize_ident(target_table_name); + + // Check all trigger commands for INSERT/UPDATE targeting the table being renamed + for cmd in &trigger.commands { + match cmd { + ast::TriggerCmd::Insert { + tbl_name, + col_names, + .. + } => { + // Check if INSERT targets the table being renamed + let insert_table_name_norm = normalize_ident(tbl_name.as_str()); + if insert_table_name_norm == target_table_name_norm { + // Check if column name appears in INSERT column list + for col_name in col_names { + let col_norm = normalize_ident(col_name.as_str()); + if col_norm == column_name_norm { + return Ok(true); + } + } + } + } + ast::TriggerCmd::Update { tbl_name, sets, .. } => { + // Check if UPDATE targets the table being renamed + let update_table_name_norm = normalize_ident(tbl_name.as_str()); + if update_table_name_norm == target_table_name_norm { + // Check if column name appears in UPDATE SET column list + for set in sets { + for col_name in &set.col_names { + let col_norm = normalize_ident(col_name.as_str()); + if col_norm == column_name_norm { + return Ok(true); + } + } + } + } + } + _ => {} + } + } + + Ok(false) +} + +/// Walk through all expressions in a SELECT statement +fn walk_select_expressions( + select: &ast::Select, + table: &crate::schema::BTreeTable, + trigger_table_name: &str, + column_name_norm: &str, + found: &mut bool, +) -> Result<()> { + // Check WITH clause (CTEs) + if let Some(ref with_clause) = select.with { + for cte in &with_clause.ctes { + walk_select_expressions( + &cte.select, + table, + trigger_table_name, + column_name_norm, + found, + )?; + if *found { + return Ok(()); + } + } + } + + // Check main SELECT body + walk_one_select_expressions( + &select.body.select, + table, + trigger_table_name, + column_name_norm, + found, + )?; + if *found { + return Ok(()); + } + + // Check compound SELECTs (UNION, EXCEPT, INTERSECT) + for compound in &select.body.compounds { + walk_one_select_expressions( + &compound.select, + table, + trigger_table_name, + column_name_norm, + found, + )?; + if *found { + return Ok(()); + } + } + + // Check ORDER BY + for sorted_col in &select.order_by { + walk_expr( + &sorted_col.expr, + &mut |e: &ast::Expr| -> Result { + check_column_ref(e, table, trigger_table_name, column_name_norm, found)?; + Ok(WalkControl::Continue) + }, + )?; + if *found { + return Ok(()); + } + } + + // Check LIMIT + if let Some(ref limit) = select.limit { + walk_expr(&limit.expr, &mut |e: &ast::Expr| -> Result { + check_column_ref(e, table, trigger_table_name, column_name_norm, found)?; + Ok(WalkControl::Continue) + })?; + if *found { + return Ok(()); + } + if let Some(ref offset) = limit.offset { + walk_expr(offset, &mut |e: &ast::Expr| -> Result { + check_column_ref(e, table, trigger_table_name, column_name_norm, found)?; + Ok(WalkControl::Continue) + })?; + if *found { + return Ok(()); + } + } + } + + Ok(()) +} + +/// Walk through all expressions in a OneSelect +fn walk_one_select_expressions( + one_select: &ast::OneSelect, + table: &crate::schema::BTreeTable, + trigger_table_name: &str, + column_name_norm: &str, + found: &mut bool, +) -> Result<()> { + match one_select { + ast::OneSelect::Select { + columns, + from, + where_clause, + group_by, + window_clause, + .. + } => { + // Check columns + for col in columns { + if let ast::ResultColumn::Expr(expr, _) = col { + walk_expr(expr, &mut |e: &ast::Expr| -> Result { + check_column_ref(e, table, trigger_table_name, column_name_norm, found)?; + Ok(WalkControl::Continue) + })?; + if *found { + return Ok(()); + } + } + } + + // Check FROM clause and JOIN conditions + if let Some(ref from_clause) = from { + walk_from_clause_expressions( + from_clause, + table, + trigger_table_name, + column_name_norm, + found, + )?; + if *found { + return Ok(()); + } + } + + // Check WHERE clause + if let Some(ref where_expr) = where_clause { + walk_expr(where_expr, &mut |e: &ast::Expr| -> Result { + check_column_ref(e, table, trigger_table_name, column_name_norm, found)?; + Ok(WalkControl::Continue) + })?; + if *found { + return Ok(()); + } + } + + // Check GROUP BY and HAVING + if let Some(ref group_by) = group_by { + for expr in &group_by.exprs { + walk_expr(expr, &mut |e: &ast::Expr| -> Result { + check_column_ref(e, table, trigger_table_name, column_name_norm, found)?; + Ok(WalkControl::Continue) + })?; + if *found { + return Ok(()); + } + } + if let Some(ref having_expr) = group_by.having { + walk_expr(having_expr, &mut |e: &ast::Expr| -> Result { + check_column_ref(e, table, trigger_table_name, column_name_norm, found)?; + Ok(WalkControl::Continue) + })?; + if *found { + return Ok(()); + } + } + } + + // Check WINDOW clause + for window_def in window_clause { + walk_window_expressions( + &window_def.window, + table, + trigger_table_name, + column_name_norm, + found, + )?; + if *found { + return Ok(()); + } + } + } + ast::OneSelect::Values(values) => { + for row in values { + for expr in row { + walk_expr(expr, &mut |e: &ast::Expr| -> Result { + check_column_ref(e, table, trigger_table_name, column_name_norm, found)?; + Ok(WalkControl::Continue) + })?; + if *found { + return Ok(()); + } + } + } + } + } + Ok(()) +} + +/// Walk through expressions in a FROM clause (including JOIN conditions) +fn walk_from_clause_expressions( + from_clause: &ast::FromClause, + table: &crate::schema::BTreeTable, + trigger_table_name: &str, + column_name_norm: &str, + found: &mut bool, +) -> Result<()> { + // Check main table (could be a subquery) + walk_select_table_expressions( + &from_clause.select, + table, + trigger_table_name, + column_name_norm, + found, + )?; + if *found { + return Ok(()); + } + + // Check JOIN conditions + for join in &from_clause.joins { + walk_select_table_expressions( + &join.table, + table, + trigger_table_name, + column_name_norm, + found, + )?; + if *found { + return Ok(()); + } + if let Some(ref constraint) = join.constraint { + match constraint { + ast::JoinConstraint::On(expr) => { + walk_expr(expr, &mut |e: &ast::Expr| -> Result { + check_column_ref(e, table, trigger_table_name, column_name_norm, found)?; + Ok(WalkControl::Continue) + })?; + if *found { + return Ok(()); + } + } + ast::JoinConstraint::Using(_) => { + // USING clause contains column names, not expressions + } + } + } + } + Ok(()) +} + +/// Walk through expressions in a SelectTable (table, subquery, or table function) +fn walk_select_table_expressions( + select_table: &ast::SelectTable, + table: &crate::schema::BTreeTable, + trigger_table_name: &str, + column_name_norm: &str, + found: &mut bool, +) -> Result<()> { + match select_table { + ast::SelectTable::Select(select, _) => { + walk_select_expressions(select, table, trigger_table_name, column_name_norm, found)?; + } + ast::SelectTable::Sub(from_clause, _) => { + walk_from_clause_expressions( + from_clause, + table, + trigger_table_name, + column_name_norm, + found, + )?; + } + ast::SelectTable::TableCall(_, args, _) => { + for arg in args { + walk_expr(arg, &mut |e: &ast::Expr| -> Result { + check_column_ref(e, table, trigger_table_name, column_name_norm, found)?; + Ok(WalkControl::Continue) + })?; + if *found { + return Ok(()); + } + } + } + ast::SelectTable::Table(_, _, _) => { + // Table reference, no expressions + } + } + Ok(()) +} + +/// Walk through expressions in a Window definition +fn walk_window_expressions( + window: &ast::Window, + table: &crate::schema::BTreeTable, + trigger_table_name: &str, + column_name_norm: &str, + found: &mut bool, +) -> Result<()> { + // Check PARTITION BY expressions + for expr in &window.partition_by { + walk_expr(expr, &mut |e: &ast::Expr| -> Result { + check_column_ref(e, table, trigger_table_name, column_name_norm, found)?; + Ok(WalkControl::Continue) + })?; + if *found { + return Ok(()); + } + } + + // Check ORDER BY expressions + for sorted_col in &window.order_by { + walk_expr( + &sorted_col.expr, + &mut |e: &ast::Expr| -> Result { + check_column_ref(e, table, trigger_table_name, column_name_norm, found)?; + Ok(WalkControl::Continue) + }, + )?; + if *found { + return Ok(()); + } + } + + // TODO: FrameClause can also contain expressions, but they're more complex + // For now, we'll skip them as they're less common in triggers + Ok(()) +} + +/// Rewrite trigger SQL to replace old column name with new column name. +/// This handles old.x, new.x, and unqualified x references. +fn rewrite_trigger_sql_for_column_rename( + trigger_sql: &str, + table_name: &str, + old_column_name: &str, + new_column_name: &str, + resolver: &Resolver, +) -> Result { + use turso_parser::parser::Parser; + + // Parse the trigger SQL + let mut parser = Parser::new(trigger_sql.as_bytes()); + let cmd = parser + .next_cmd() + .map_err(|e| LimboError::ParseError(format!("failed to parse trigger SQL: {e}")))?; + let Some(ast::Cmd::Stmt(ast::Stmt::CreateTrigger { + temporary, + if_not_exists, + trigger_name, + time, + event, + tbl_name, + for_each_row, + when_clause, + commands, + })) = cmd + else { + return Err(LimboError::ParseError(format!( + "failed to parse trigger SQL: {trigger_sql}" + ))); + }; + + let old_col_norm = normalize_ident(old_column_name); + let new_col_norm = normalize_ident(new_column_name); + + // Get the trigger's owning table to check unqualified column references + let trigger_table_name_raw = tbl_name.name.as_str(); + let trigger_table_name = normalize_ident(trigger_table_name_raw); + let trigger_table = resolver + .schema + .get_btree_table(&trigger_table_name) + .ok_or_else(|| { + LimboError::ParseError(format!("trigger table not found: {trigger_table_name}")) + })?; + + // Check if this trigger references the column being renamed + // We need to check if the column exists in the table being renamed + let target_table_name = normalize_ident(table_name); + let target_table = resolver + .schema + .get_btree_table(&target_table_name) + .ok_or_else(|| { + LimboError::ParseError(format!("target table not found: {target_table_name}")) + })?; + + // Rewrite UPDATE OF column list if renaming a column in the trigger's owning table + let is_renaming_trigger_table = trigger_table_name == target_table_name; + let new_event = if is_renaming_trigger_table { + match event { + ast::TriggerEvent::UpdateOf(mut cols) => { + // Rewrite column names in UPDATE OF list + for col in &mut cols { + let col_norm = normalize_ident(col.as_str()); + if col_norm == old_col_norm { + *col = ast::Name::from_string(new_col_norm.clone()); + } + } + ast::TriggerEvent::UpdateOf(cols) + } + other => other, + } + } else { + event + }; + + // 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 { + let new_cmd = rewrite_trigger_cmd_for_column_rename( + cmd, + &trigger_table, + &target_table, + trigger_table_name_raw, + &target_table_name, + &old_col_norm, + &new_col_norm, + resolver, + )?; + new_commands.push(new_cmd); + } + + // Reconstruct the SQL + use crate::translate::trigger::create_trigger_to_sql; + let new_sql = create_trigger_to_sql( + temporary, + if_not_exists, + &trigger_name, + time, + &new_event, + &tbl_name, + for_each_row, + new_when_clause.as_deref(), + &new_commands, + ); + + Ok(new_sql) +} + +/// Rewrite an expression to replace column references +/// +/// `context_table_name` is used for UPDATE/DELETE WHERE clauses where unqualified column +/// references refer to the UPDATE/DELETE target table, not the trigger's owning table. +/// If `None`, unqualified references refer to the trigger's owning table. +#[allow(clippy::too_many_arguments)] +fn rewrite_expr_for_column_rename( + expr: &ast::Expr, + trigger_table: &crate::schema::BTreeTable, + trigger_table_name: &str, + target_table_name: &str, + old_col_norm: &str, + new_col_norm: &str, + context_table_name: Option<&str>, + resolver: &Resolver, +) -> Result { + use crate::translate::expr::walk_expr_mut; + + let trigger_table_name_norm = normalize_ident(trigger_table_name); + let target_table_name_norm = normalize_ident(target_table_name); + let is_renaming_trigger_table = trigger_table_name_norm == target_table_name_norm; + + // Get context table if provided (for UPDATE/DELETE WHERE clauses) + let context_table_info: Option<(std::sync::Arc, String, bool)> = + if let Some(ctx_name) = context_table_name { + let ctx_name_norm = normalize_ident(ctx_name); + let is_renaming = ctx_name_norm == target_table_name_norm; + let table = resolver + .schema + .get_btree_table(&ctx_name_norm) + .ok_or_else(|| { + LimboError::ParseError(format!("context table not found: {ctx_name_norm}")) + })?; + Some((table, ctx_name_norm, is_renaming)) + } else { + None + }; + + let mut expr = expr.clone(); + walk_expr_mut(&mut expr, &mut |e: &mut ast::Expr| -> Result { + rewrite_expr_column_ref_with_context( + e, + trigger_table, + trigger_table_name, + old_col_norm, + new_col_norm, + is_renaming_trigger_table, + context_table_info + .as_ref() + .map(|(t, n, r)| (t.as_ref(), n, *r)), + )?; + Ok(WalkControl::Continue) + })?; + + Ok(expr) +} + +/// Rewrite a trigger command to replace column references +#[allow(clippy::too_many_arguments)] +fn rewrite_trigger_cmd_for_column_rename( + cmd: ast::TriggerCmd, + trigger_table: &crate::schema::BTreeTable, + target_table: &crate::schema::BTreeTable, + trigger_table_name: &str, + target_table_name: &str, + old_col_norm: &str, + new_col_norm: &str, + resolver: &Resolver, +) -> Result { + use crate::translate::expr::walk_expr_mut; + + match cmd { + ast::TriggerCmd::Update { + or_conflict, + tbl_name, + mut sets, + from, + where_clause, + } => { + // Get the UPDATE target table to check if we're renaming a column in it + let update_table_name_norm = normalize_ident(tbl_name.as_str()); + let is_renaming_update_table = update_table_name_norm == *target_table_name; + + // Rewrite SET column names if renaming a column in the UPDATE target table + if is_renaming_update_table { + for set in &mut sets { + for col_name in &mut set.col_names { + let col_norm = normalize_ident(col_name.as_str()); + if col_norm == *old_col_norm { + *col_name = ast::Name::from_string(new_col_norm); + } + } + } + } + + // Rewrite SET expressions + for set in &mut sets { + walk_expr_mut( + &mut set.expr, + &mut |e: &mut ast::Expr| -> Result { + rewrite_expr_column_ref( + e, + trigger_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + Ok(WalkControl::Continue) + }, + )?; + } + + // Rewrite WHERE clause - unqualified column references refer to UPDATE target table + let new_where = where_clause + .map(|e| { + rewrite_expr_for_column_rename( + &e, + trigger_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + Some(&update_table_name_norm), // UPDATE WHERE: unqualified refs refer to UPDATE target + resolver, + ) + .map(Box::new) + }) + .transpose()?; + Ok(ast::TriggerCmd::Update { + or_conflict, + tbl_name, + sets, + from, + where_clause: new_where, + }) + } + ast::TriggerCmd::Insert { + or_conflict, + tbl_name, + mut col_names, + select, + upsert, + returning, + } => { + // Rewrite column names in INSERT column list + // Check if the INSERT is targeting the table being renamed + let insert_table_name_norm = normalize_ident(tbl_name.as_str()); + if insert_table_name_norm == *target_table_name { + // This INSERT targets the table being renamed, so rewrite column names + for col_name in &mut col_names { + let col_norm = normalize_ident(col_name.as_str()); + if col_norm == *old_col_norm { + *col_name = ast::Name::from_string(new_col_norm); + } + } + } + // Rewrite SELECT expressions + let new_select = rewrite_select_for_column_rename( + select, + trigger_table, + target_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + Ok(ast::TriggerCmd::Insert { + or_conflict, + tbl_name, + col_names, + select: new_select, + upsert, + returning, + }) + } + ast::TriggerCmd::Delete { + tbl_name, + where_clause, + } => { + // Get the DELETE target table to check if we're renaming a column in it + let delete_table_name_norm = normalize_ident(tbl_name.as_str()); + + // Rewrite WHERE clause - unqualified column references refer to DELETE target table + let new_where = where_clause + .map(|e| { + rewrite_expr_for_column_rename( + &e, + trigger_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + Some(&delete_table_name_norm), // DELETE WHERE: unqualified refs refer to DELETE target + resolver, + ) + .map(Box::new) + }) + .transpose()?; + Ok(ast::TriggerCmd::Delete { + tbl_name, + where_clause: new_where, + }) + } + ast::TriggerCmd::Select(select) => { + let new_select = rewrite_select_for_column_rename( + select, + trigger_table, + target_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + Ok(ast::TriggerCmd::Select(new_select)) + } + } +} + +/// Rewrite a SELECT statement to replace column references +fn rewrite_select_for_column_rename( + select: ast::Select, + trigger_table: &crate::schema::BTreeTable, + _target_table: &crate::schema::BTreeTable, + trigger_table_name: &str, + target_table_name: &str, + old_col_norm: &str, + new_col_norm: &str, +) -> Result { + use crate::translate::expr::walk_expr_mut; + + let mut rewrite_cb = |e: &mut ast::Expr| -> Result { + rewrite_expr_column_ref( + e, + trigger_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + Ok(WalkControl::Continue) + }; + + let mut select = select; + + // Rewrite WITH clause (CTEs) + // Note: We clone here because Select doesn't implement Default, so we can't use mem::take. + // The clone is necessary to move ownership to rewrite_select_for_column_rename. + if let Some(ref mut with_clause) = select.with { + for cte in &mut with_clause.ctes { + cte.select = rewrite_select_for_column_rename( + cte.select.clone(), + trigger_table, + _target_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + } + } + + // Rewrite main SELECT body + rewrite_one_select_for_column_rename( + &mut select.body.select, + trigger_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + + // Rewrite compound SELECTs (UNION, EXCEPT, INTERSECT) + for compound in &mut select.body.compounds { + rewrite_one_select_for_column_rename( + &mut compound.select, + trigger_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + } + + // Rewrite ORDER BY + for sorted_col in &mut select.order_by { + walk_expr_mut(&mut sorted_col.expr, &mut rewrite_cb)?; + } + + // Rewrite LIMIT + if let Some(ref mut limit) = select.limit { + walk_expr_mut(&mut limit.expr, &mut rewrite_cb)?; + if let Some(ref mut offset) = limit.offset { + walk_expr_mut(offset, &mut rewrite_cb)?; + } + } + + Ok(select) +} + +/// Rewrite a OneSelect to replace column references +fn rewrite_one_select_for_column_rename( + one_select: &mut ast::OneSelect, + trigger_table: &crate::schema::BTreeTable, + trigger_table_name: &str, + target_table_name: &str, + old_col_norm: &str, + new_col_norm: &str, +) -> Result<()> { + use crate::translate::expr::walk_expr_mut; + + let mut rewrite_cb = |e: &mut ast::Expr| -> Result { + rewrite_expr_column_ref( + e, + trigger_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + Ok(WalkControl::Continue) + }; + + match one_select { + ast::OneSelect::Select { + columns, + from, + where_clause, + group_by, + window_clause, + .. + } => { + // Rewrite columns + for col in columns { + if let ast::ResultColumn::Expr(expr, _) = col { + walk_expr_mut(expr, &mut rewrite_cb)?; + } + } + + // Rewrite FROM clause and JOIN conditions + if let Some(ref mut from_clause) = from { + rewrite_from_clause_for_column_rename( + from_clause, + trigger_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + } + + // Rewrite WHERE clause + if let Some(ref mut where_expr) = where_clause { + walk_expr_mut(where_expr, &mut rewrite_cb)?; + } + + // Rewrite GROUP BY and HAVING + if let Some(ref mut group_by) = group_by { + for expr in &mut group_by.exprs { + walk_expr_mut(expr, &mut rewrite_cb)?; + } + if let Some(ref mut having_expr) = group_by.having { + walk_expr_mut(having_expr, &mut rewrite_cb)?; + } + } + + // Rewrite WINDOW clause + for window_def in window_clause { + rewrite_window_for_column_rename( + &mut window_def.window, + trigger_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + } + } + ast::OneSelect::Values(values) => { + for row in values { + for expr in row { + walk_expr_mut(expr, &mut rewrite_cb)?; + } + } + } + } + Ok(()) +} + +/// Rewrite expressions in a FROM clause (including JOIN conditions) +fn rewrite_from_clause_for_column_rename( + from_clause: &mut ast::FromClause, + trigger_table: &crate::schema::BTreeTable, + trigger_table_name: &str, + target_table_name: &str, + old_col_norm: &str, + new_col_norm: &str, +) -> Result<()> { + use crate::translate::expr::walk_expr_mut; + + let mut rewrite_cb = |e: &mut ast::Expr| -> Result { + rewrite_expr_column_ref( + e, + trigger_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + Ok(WalkControl::Continue) + }; + + // Rewrite main table (could be a subquery) + rewrite_select_table_for_column_rename( + &mut from_clause.select, + trigger_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + + // Rewrite JOIN conditions + for join in &mut from_clause.joins { + rewrite_select_table_for_column_rename( + &mut join.table, + trigger_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + if let Some(ref mut constraint) = join.constraint { + match constraint { + ast::JoinConstraint::On(expr) => { + walk_expr_mut(expr, &mut rewrite_cb)?; + } + ast::JoinConstraint::Using(_) => { + // USING clause contains column names, not expressions + } + } + } + } + Ok(()) +} + +/// Rewrite expressions in a SelectTable (table, subquery, or table function) +fn rewrite_select_table_for_column_rename( + select_table: &mut ast::SelectTable, + trigger_table: &crate::schema::BTreeTable, + trigger_table_name: &str, + target_table_name: &str, + old_col_norm: &str, + new_col_norm: &str, +) -> Result<()> { + use crate::translate::expr::walk_expr_mut; + + let mut rewrite_cb = |e: &mut ast::Expr| -> Result { + rewrite_expr_column_ref( + e, + trigger_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + Ok(WalkControl::Continue) + }; + + match select_table { + ast::SelectTable::Select(select, _) => { + // Note: We clone here because Select doesn't implement Default, so we can't use mem::take. + // The clone is necessary to move ownership to rewrite_select_for_column_rename. + *select = rewrite_select_for_column_rename( + select.clone(), + trigger_table, + trigger_table, // target_table not needed for subqueries + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + } + ast::SelectTable::Sub(from_clause, _) => { + rewrite_from_clause_for_column_rename( + from_clause, + trigger_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + } + ast::SelectTable::TableCall(_, args, _) => { + for arg in args { + walk_expr_mut(arg, &mut rewrite_cb)?; + } + } + ast::SelectTable::Table(_, _, _) => { + // Table reference, no expressions + } + } + Ok(()) +} + +/// Rewrite expressions in a Window definition +fn rewrite_window_for_column_rename( + window: &mut ast::Window, + trigger_table: &crate::schema::BTreeTable, + trigger_table_name: &str, + target_table_name: &str, + old_col_norm: &str, + new_col_norm: &str, +) -> Result<()> { + use crate::translate::expr::walk_expr_mut; + + let mut rewrite_cb = |e: &mut ast::Expr| -> Result { + rewrite_expr_column_ref( + e, + trigger_table, + trigger_table_name, + target_table_name, + old_col_norm, + new_col_norm, + )?; + Ok(WalkControl::Continue) + }; + + // Rewrite PARTITION BY expressions + for expr in &mut window.partition_by { + walk_expr_mut(expr, &mut rewrite_cb)?; + } + + // Rewrite ORDER BY expressions + for sorted_col in &mut window.order_by { + walk_expr_mut(&mut sorted_col.expr, &mut rewrite_cb)?; + } + + // TODO: FrameClause can also contain expressions, but they're more complex + // For now, we'll skip them as they're less common in triggers + Ok(()) +} + +/// Rewrite a single expression's column reference +/// +/// Handles column references in trigger expressions: +/// - NEW.column and OLD.column: Always refer to the trigger's owning table +/// - Qualified references (e.g., u.x): Refer to the specified table +/// - Unqualified references (e.g., x): Resolution order: +/// 1. If `context_table` is provided (UPDATE/DELETE WHERE clauses), check the context table first +/// 2. Otherwise, check the trigger's owning table +/// +/// This matches SQLite's column resolution order where unqualified columns in UPDATE/DELETE +/// WHERE clauses refer to the target table, not the trigger's owning table. +/// +/// `context_table`: Optional tuple of (table, normalized_name, is_renaming) for UPDATE/DELETE +/// target tables. If `None`, unqualified references refer to the trigger's owning table. +fn rewrite_expr_column_ref_with_context( + e: &mut ast::Expr, + trigger_table: &crate::schema::BTreeTable, + trigger_table_name: &str, + old_col_norm: &str, + new_col_norm: &str, + is_renaming_trigger_table: bool, + context_table: Option<(&crate::schema::BTreeTable, &String, bool)>, +) -> Result<()> { + 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 if this is NEW.column or OLD.column + if (ns_norm.eq_ignore_ascii_case("new") || ns_norm.eq_ignore_ascii_case("old")) + && col_norm == *old_col_norm + { + // NEW.x and OLD.x always refer to the trigger's owning table + if is_renaming_trigger_table && trigger_table.get_column(&col_norm).is_some() { + *col = ast::Name::from_string(new_col_norm); + } + } else if col_norm == *old_col_norm { + // This is a qualified column reference like u.x or t.x + // Check if it refers to the context table (UPDATE/DELETE target) or trigger table + if let Some((_, ctx_name_norm, is_renaming_ctx)) = context_table { + if ns_norm == *ctx_name_norm && is_renaming_ctx { + // Qualified reference to context table (e.g., u.x where u is UPDATE target) + *col = ast::Name::from_string(new_col_norm); + } + } + // Also check if it's a qualified reference to the trigger's owning table + // (e.g., t.x in a trigger on table t) + if is_renaming_trigger_table { + let trigger_table_name_norm = normalize_ident(trigger_table_name); + if ns_norm == trigger_table_name_norm + && trigger_table.get_column(&col_norm).is_some() + { + *col = ast::Name::from_string(new_col_norm); + } + } + } + } + ast::Expr::Id(col) => { + // Unqualified column reference + let col_norm = normalize_ident(col.as_str()); + if col_norm == *old_col_norm { + // Check context table first (for UPDATE/DELETE WHERE clauses) + if let Some((ctx_table, _, is_renaming_ctx)) = context_table { + if ctx_table.get_column(&col_norm).is_some() { + // This refers to the context table (UPDATE/DELETE target) + if is_renaming_ctx { + *e = ast::Expr::Id(ast::Name::from_string(new_col_norm)); + } + return Ok(()); + } + } + // Otherwise, check trigger's owning table + if is_renaming_trigger_table && trigger_table.get_column(&col_norm).is_some() { + *e = ast::Expr::Id(ast::Name::from_string(new_col_norm)); + } + } + } + _ => {} + } + Ok(()) +} + +/// Rewrite a single expression's column reference (convenience wrapper for non-context cases) +fn rewrite_expr_column_ref( + e: &mut ast::Expr, + trigger_table: &crate::schema::BTreeTable, + trigger_table_name: &str, + target_table_name: &str, + old_col_norm: &str, + new_col_norm: &str, +) -> Result<()> { + let trigger_table_name_norm = normalize_ident(trigger_table_name); + let target_table_name_norm = normalize_ident(target_table_name); + let is_renaming_trigger_table = trigger_table_name_norm == target_table_name_norm; + + rewrite_expr_column_ref_with_context( + e, + trigger_table, + trigger_table_name, + old_col_norm, + new_col_norm, + is_renaming_trigger_table, + None, + ) +} diff --git a/tests/integration/trigger.rs b/tests/integration/trigger.rs index 22122e0b7..1898de5da 100644 --- a/tests/integration/trigger.rs +++ b/tests/integration/trigger.rs @@ -886,3 +886,1070 @@ fn test_trigger_with_multiple_statements() { assert_eq!(audit_results[0], "Balance changed for account 1"); assert_eq!(audit_results[1], "Balance changed for account 2"); } + +#[test] +fn test_alter_table_drop_column_fails_when_trigger_references_new_column() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create table with columns + conn.execute("CREATE TABLE t (x INTEGER PRIMARY KEY, y INTEGER)") + .unwrap(); + + // Create trigger that references y via NEW.y + conn.execute( + "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN + INSERT INTO t VALUES (NEW.x, NEW.y); + END", + ) + .unwrap(); + + // Attempting to drop column y should fail because trigger references it + let result = conn.execute("ALTER TABLE t DROP COLUMN y"); + assert!( + result.is_err(), + "Dropping column y should fail when trigger references NEW.y" + ); + + let error_msg = result.unwrap_err().to_string(); + assert!( + error_msg.contains("cannot drop column") && error_msg.contains("trigger"), + "Error should mention column drop and trigger: {error_msg}", + ); +} + +#[test] +fn test_alter_table_drop_column_fails_when_trigger_references_old_column() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create table with columns + conn.execute("CREATE TABLE t (x INTEGER PRIMARY KEY, y INTEGER)") + .unwrap(); + + // Create trigger that references y via OLD.y + conn.execute( + "CREATE TRIGGER tu AFTER UPDATE ON t BEGIN + INSERT INTO t VALUES (OLD.x, OLD.y); + END", + ) + .unwrap(); + + // Attempting to drop column y should fail because trigger references it + let result = conn.execute("ALTER TABLE t DROP COLUMN y"); + assert!( + result.is_err(), + "Dropping column y should fail when trigger references OLD.y" + ); + + let error_msg = result.unwrap_err().to_string(); + assert!( + error_msg.contains("cannot drop column") && error_msg.contains("trigger"), + "Error should mention column drop and trigger: {error_msg}", + ); +} + +#[test] +fn test_alter_table_drop_column_fails_when_trigger_references_unqualified_column() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create table with columns + conn.execute("CREATE TABLE t (x INTEGER PRIMARY KEY, y INTEGER)") + .unwrap(); + + // Create trigger that references y as unqualified column (in WHEN clause) + conn.execute( + "CREATE TRIGGER tu BEFORE INSERT ON t WHEN y > 10 BEGIN + INSERT INTO t VALUES (NEW.x, 100); + END", + ) + .unwrap(); + + // Attempting to drop column y should fail because trigger references it + let result = conn.execute("ALTER TABLE t DROP COLUMN y"); + assert!( + result.is_err(), + "Dropping column y should fail when trigger references it in WHEN clause" + ); + + let error_msg = result.unwrap_err().to_string(); + assert!( + error_msg.contains("cannot drop column") && error_msg.contains("trigger"), + "Error should mention column drop and trigger: {error_msg}", + ); +} + +#[test] +fn test_alter_table_drop_column_succeeds_when_trigger_references_other_table() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create two tables + conn.execute("CREATE TABLE t (x INTEGER PRIMARY KEY, y INTEGER)") + .unwrap(); + conn.execute("CREATE TABLE u (z INTEGER)").unwrap(); + + // Create trigger on t that references column from u (not t) + conn.execute( + "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN + INSERT INTO u(z) VALUES (NEW.x); + END", + ) + .unwrap(); + + // Dropping column y from t should succeed because trigger doesn't reference it + conn.execute("ALTER TABLE t DROP COLUMN y").unwrap(); + + // Verify column was dropped + let mut stmt = conn.prepare("PRAGMA table_info(t)").unwrap(); + let mut columns = Vec::new(); + loop { + match stmt.step().unwrap() { + turso_core::StepResult::Row => { + let row = stmt.row().unwrap(); + columns.push(row.get_value(1).cast_text().unwrap().to_string()); + } + turso_core::StepResult::Done => break, + turso_core::StepResult::IO => { + stmt.run_once().unwrap(); + } + _ => panic!("Unexpected step result"), + } + } + + // Should only have x column now + assert_eq!(columns.len(), 1); + assert_eq!(columns[0], "x"); +} + +#[test] +fn test_alter_table_drop_column_from_other_table_causes_parse_error_when_trigger_fires() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create two tables + conn.execute("CREATE TABLE t (x INTEGER PRIMARY KEY)") + .unwrap(); + conn.execute("CREATE TABLE u (z INTEGER, zer INTEGER)") + .unwrap(); + + // Create trigger on t that references column zer from u + conn.execute( + "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN + INSERT INTO u(z, zer) VALUES (NEW.x, NEW.x); + END", + ) + .unwrap(); + + // Dropping column zer from u should succeed (trigger is on t, not u) + conn.execute("ALTER TABLE u DROP COLUMN zer").unwrap(); + + // Verify column was dropped + let mut stmt = conn.prepare("PRAGMA table_info(u)").unwrap(); + let mut columns = Vec::new(); + loop { + match stmt.step().unwrap() { + turso_core::StepResult::Row => { + let row = stmt.row().unwrap(); + columns.push(row.get_value(1).cast_text().unwrap().to_string()); + } + turso_core::StepResult::Done => break, + turso_core::StepResult::IO => { + stmt.run_once().unwrap(); + } + _ => panic!("Unexpected step result"), + } + } + + // Should only have z column now + assert_eq!(columns.len(), 1); + assert_eq!(columns[0], "z"); + + // Now trying to insert into t should fail because trigger references non-existent column zer + let result = conn.execute("INSERT INTO t VALUES (1)"); + assert!( + result.is_err(), + "Insert should fail because trigger references non-existent column zer" + ); + + let error_msg = result.unwrap_err().to_string(); + assert!( + error_msg.contains("no column named") || error_msg.contains("zer"), + "Error should mention missing column: {error_msg}", + ); +} + +#[test] +fn test_alter_table_rename_column_propagates_to_trigger_on_owning_table() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create table + conn.execute("CREATE TABLE t (x INTEGER PRIMARY KEY, y INTEGER)") + .unwrap(); + + // Create trigger on t that references y via NEW.y + conn.execute( + "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN + INSERT INTO t VALUES (NEW.x, NEW.y); + END", + ) + .unwrap(); + + // Rename column y to y_new + conn.execute("ALTER TABLE t RENAME COLUMN y TO y_new") + .unwrap(); + + // 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 should reference y_new instead of y + let normalized_sql = trigger_sql.split_whitespace().collect::>().join(" "); + assert_eq!( + normalized_sql, + "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN INSERT INTO t VALUES (NEW.x, NEW.y_new); END" + ); +} + +#[test] +fn test_alter_table_rename_column_propagates_to_trigger_referencing_other_table() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create two tables + conn.execute("CREATE TABLE t (x INTEGER PRIMARY KEY)") + .unwrap(); + conn.execute("CREATE TABLE u (z INTEGER, zer INTEGER)") + .unwrap(); + + // Create trigger on t that references column z from u + conn.execute( + "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN + INSERT INTO u(z, zer) VALUES (NEW.x, NEW.x); + END", + ) + .unwrap(); + + // Rename column z to zoo in table u + conn.execute("ALTER TABLE u RENAME COLUMN z TO zoo") + .unwrap(); + + // Verify trigger SQL was updated to reference zoo + 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 should reference zoo instead of z + let normalized_sql = trigger_sql.split_whitespace().collect::>().join(" "); + assert_eq!( + normalized_sql, + "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN INSERT INTO u (zoo, zer) VALUES (NEW.x, NEW.x); END" + ); +} + +#[test] +fn test_alter_table_rename_column_propagates_to_trigger_with_multiple_references() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create two tables + conn.execute("CREATE TABLE t (x INTEGER PRIMARY KEY, y INTEGER)") + .unwrap(); + conn.execute("CREATE TABLE u (z INTEGER, zer INTEGER)") + .unwrap(); + + // Create trigger on t that references y multiple times and z from u + conn.execute( + "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN + INSERT INTO u(z, zer) VALUES (NEW.y, NEW.y); + END", + ) + .unwrap(); + + // Rename column y to y_new in table t + conn.execute("ALTER TABLE t RENAME COLUMN y TO y_new") + .unwrap(); + + // 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 should reference y_new instead of y + let normalized_sql = trigger_sql.split_whitespace().collect::>().join(" "); + assert_eq!( + normalized_sql, + "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN INSERT INTO u (z, zer) VALUES (NEW.y_new, NEW.y_new); END" + ); +} + +#[test] +fn test_alter_table_rename_column_fails_when_trigger_when_clause_references_column() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create table + conn.execute("CREATE TABLE t (x INTEGER PRIMARY KEY, y INTEGER)") + .unwrap(); + + // Create trigger with WHEN clause referencing y + conn.execute( + "CREATE TRIGGER tu BEFORE INSERT ON t WHEN y > 10 BEGIN + INSERT INTO t VALUES (NEW.x, 100); + END", + ) + .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" + ); + + 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}", + ); +} + +#[test] +fn test_alter_table_rename_column_propagates_to_multiple_triggers() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create table + conn.execute("CREATE TABLE t (x INTEGER PRIMARY KEY, y INTEGER)") + .unwrap(); + + // Create multiple triggers referencing y + conn.execute( + "CREATE TRIGGER t1 BEFORE INSERT ON t BEGIN + INSERT INTO t VALUES (NEW.x, NEW.y); + END", + ) + .unwrap(); + + conn.execute( + "CREATE TRIGGER t2 AFTER UPDATE ON t BEGIN + INSERT INTO t VALUES (OLD.x, OLD.y); + END", + ) + .unwrap(); + + // Rename column y to y_new + conn.execute("ALTER TABLE t RENAME COLUMN y TO y_new") + .unwrap(); + + // Verify both triggers were updated + let mut stmt = conn + .prepare("SELECT sql FROM sqlite_schema WHERE type='trigger' ORDER BY name") + .unwrap(); + let mut trigger_sqls = Vec::new(); + loop { + match stmt.step().unwrap() { + turso_core::StepResult::Row => { + let row = stmt.row().unwrap(); + trigger_sqls.push(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"), + } + } + + // Both triggers should reference y_new + assert_eq!(trigger_sqls.len(), 2); + let normalized_t1 = trigger_sqls[0] + .split_whitespace() + .collect::>() + .join(" "); + let normalized_t2 = trigger_sqls[1] + .split_whitespace() + .collect::>() + .join(" "); + assert_eq!( + normalized_t1, + "CREATE TRIGGER t1 BEFORE INSERT ON t BEGIN INSERT INTO t VALUES (NEW.x, NEW.y_new); END" + ); + assert_eq!( + normalized_t2, + "CREATE TRIGGER t2 AFTER UPDATE ON t BEGIN INSERT INTO t VALUES (OLD.x, OLD.y_new); END" + ); +} + +#[test] +fn test_alter_table_drop_column_fails_with_old_reference_in_update_trigger() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create table + conn.execute("CREATE TABLE t (x INTEGER PRIMARY KEY, y INTEGER)") + .unwrap(); + + // Create UPDATE trigger that references OLD.y + conn.execute( + "CREATE TRIGGER tu AFTER UPDATE ON t BEGIN + INSERT INTO t VALUES (OLD.x, OLD.y); + END", + ) + .unwrap(); + + // Attempting to drop column y should fail + let result = conn.execute("ALTER TABLE t DROP COLUMN y"); + assert!( + result.is_err(), + "Dropping column y should fail when UPDATE trigger references OLD.y" + ); +} + +#[test] +fn test_alter_table_rename_column_in_insert_column_list() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create two tables + conn.execute("CREATE TABLE t (x INTEGER PRIMARY KEY)") + .unwrap(); + conn.execute("CREATE TABLE u (z INTEGER, zer INTEGER)") + .unwrap(); + + // Create trigger that inserts into u with explicit column list + conn.execute( + "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN + INSERT INTO u(z, zer) VALUES (NEW.x, NEW.x); + END", + ) + .unwrap(); + + // Rename column zer to zercher in table u + conn.execute("ALTER TABLE u RENAME COLUMN zer TO zercher") + .unwrap(); + + // 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 should reference zercher in INSERT column list + let normalized_sql = trigger_sql.split_whitespace().collect::>().join(" "); + assert_eq!( + normalized_sql, + "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN INSERT INTO u (z, zercher) VALUES (NEW.x, NEW.x); END" + ); +} + +#[test] +fn test_alter_table_rename_column_in_trigger_table_does_not_rewrite_other_table_column() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create two tables, both with column 'x' + conn.execute("CREATE TABLE t (x INTEGER PRIMARY KEY, y INTEGER)") + .unwrap(); + conn.execute("CREATE TABLE u (x INTEGER, z INTEGER)") + .unwrap(); + + // Create trigger on t that references both t.x (via NEW.x) and u.x (in INSERT column list) + conn.execute( + "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN + INSERT INTO u(x, z) VALUES (NEW.x, NEW.y); + END", + ) + .unwrap(); + + // Rename column x to x_new in table t (the trigger's owning table) + conn.execute("ALTER TABLE t RENAME COLUMN x TO x_new") + .unwrap(); + + // Verify trigger SQL was updated correctly: + // - NEW.x should become NEW.x_new (refers to table t) + // - INSERT INTO u(x, ...) should remain as x (refers to table u, not t) + 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"), + } + } + + let normalized_sql = trigger_sql.split_whitespace().collect::>().join(" "); + // NEW.x should be rewritten to NEW.x_new + assert!( + normalized_sql.contains("NEW.x_new"), + "Trigger SQL should contain NEW.x_new: {normalized_sql}", + ); + // INSERT INTO u (x, ...) should still have x (not x_new) because it refers to table u's column + assert!( + normalized_sql.contains("INSERT INTO u (x,") || normalized_sql.contains("INSERT INTO u(x,"), + "Trigger SQL should contain INSERT INTO u (x, (not u (x_new,): {normalized_sql}", + ); + assert!( + !normalized_sql.contains("INSERT INTO u (x_new,") && !normalized_sql.contains("INSERT INTO u(x_new,"), + "Trigger SQL should NOT contain INSERT INTO u (x_new, (x refers to table u): {normalized_sql}", + ); +} + +#[test] +fn test_alter_table_rename_column_in_insert_target_table_does_not_rewrite_trigger_table_column() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create two tables, both with column 'x' + conn.execute("CREATE TABLE t (x INTEGER PRIMARY KEY, y INTEGER)") + .unwrap(); + conn.execute("CREATE TABLE u (x INTEGER, z INTEGER)") + .unwrap(); + + // Create trigger on t that references both t.x (via NEW.x) and u.x (in INSERT column list) + conn.execute( + "CREATE TRIGGER tu BEFORE INSERT ON t BEGIN + INSERT INTO u(x, z) VALUES (NEW.x, NEW.y); + END", + ) + .unwrap(); + + // Rename column x to x_new in table u (the INSERT target table) + conn.execute("ALTER TABLE u RENAME COLUMN x TO x_new") + .unwrap(); + + // Verify trigger SQL was updated correctly: + // - NEW.x should remain as NEW.x (refers to table t, not u) + // - INSERT INTO u(x, ...) should become u(x_new, ...) (refers to table u) + 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"), + } + } + + let normalized_sql = trigger_sql.split_whitespace().collect::>().join(" "); + // NEW.x should remain as NEW.x (not rewritten because it refers to table t) + assert!( + normalized_sql.contains("NEW.x"), + "Trigger SQL should contain NEW.x (not NEW.x_new): {normalized_sql}", + ); + assert!( + !normalized_sql.contains("NEW.x_new"), + "Trigger SQL should NOT contain NEW.x_new (x refers to table t, not u): {normalized_sql}", + ); + // INSERT INTO u (x_new, ...) should have x_new (rewritten because it refers to table u) + assert!( + normalized_sql.contains("INSERT INTO u (x_new,") + || normalized_sql.contains("INSERT INTO u(x_new,"), + "Trigger SQL should contain INSERT INTO u (x_new, (not u (x,): {normalized_sql}", + ); + assert!( + !normalized_sql.contains("INSERT INTO u (x,") && !normalized_sql.contains("INSERT INTO u(x,"), + "Trigger SQL should NOT contain INSERT INTO u (x, (x was renamed to x_new in table u): {normalized_sql}", + ); +} + +#[test] +fn test_alter_table_rename_column_update_where_clause_does_not_rewrite_target_table_column() { + 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, with WHERE clause referencing u.x (unqualified) + conn.execute( + "CREATE TRIGGER tu AFTER UPDATE ON t BEGIN + UPDATE u SET z = NEW.x WHERE x = OLD.x; + END", + ) + .unwrap(); + + // Rename column x to x_new in table t (the trigger's owning table) + conn.execute("ALTER TABLE t RENAME COLUMN x TO x_new") + .unwrap(); + + // Verify trigger SQL: WHERE x should remain as x (refers to table u, not t) + 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"), + } + } + + let normalized_sql = trigger_sql.split_whitespace().collect::>().join(" "); + // NEW.x and OLD.x should be rewritten to x_new + assert!( + normalized_sql.contains("NEW.x_new") && normalized_sql.contains("OLD.x_new"), + "Trigger SQL should contain NEW.x_new and OLD.x_new: {normalized_sql}", + ); + // WHERE x should remain as x (not x_new) because it refers to table u's column + assert!( + normalized_sql.contains("WHERE x =") || normalized_sql.contains("WHERE x="), + "Trigger SQL should contain WHERE x = (not WHERE x_new =): {normalized_sql}", + ); + assert!( + !normalized_sql.contains("WHERE x_new =") && !normalized_sql.contains("WHERE x_new="), + "Trigger SQL should NOT contain WHERE x_new = (x refers to table u): {normalized_sql}", + ); +} + +#[test] +fn test_alter_table_rename_column_update_set_column_name_rewritten() { + 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, setting u.x + conn.execute( + "CREATE TRIGGER tu AFTER UPDATE ON t BEGIN + UPDATE u SET x = NEW.x WHERE u.x = OLD.x; + END", + ) + .unwrap(); + + // Rename column x to x_new in table u (the UPDATE target table) + conn.execute("ALTER TABLE u RENAME COLUMN x TO x_new") + .unwrap(); + + // Verify trigger SQL: SET x should become SET x_new, WHERE u.x should become u.x_new + 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"), + } + } + + let normalized_sql = trigger_sql.split_whitespace().collect::>().join(" "); + // SET x should become SET x_new + assert!( + normalized_sql.contains("SET x_new =") || normalized_sql.contains("SET x_new="), + "Trigger SQL should contain SET x_new =: {normalized_sql}", + ); + assert!( + !normalized_sql.contains("SET x =") && !normalized_sql.contains("SET x="), + "Trigger SQL should NOT contain SET x = (x was renamed to x_new): {normalized_sql}", + ); + // WHERE u.x should become u.x_new + assert!( + normalized_sql.contains("u.x_new =") || normalized_sql.contains("u.x_new="), + "Trigger SQL should contain u.x_new =: {normalized_sql}", + ); +} + +#[test] +fn test_alter_table_rename_column_delete_where_clause_does_not_rewrite_target_table_column() { + 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 deletes from u, with WHERE clause referencing u.x (unqualified) + conn.execute( + "CREATE TRIGGER tu AFTER DELETE ON t BEGIN + DELETE FROM u WHERE x = OLD.x; + END", + ) + .unwrap(); + + // Rename column x to x_new in table t (the trigger's owning table) + conn.execute("ALTER TABLE t RENAME COLUMN x TO x_new") + .unwrap(); + + // Verify trigger SQL: WHERE x should remain as x (refers to table u, not t) + 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"), + } + } + + let normalized_sql = trigger_sql.split_whitespace().collect::>().join(" "); + // OLD.x should be rewritten to OLD.x_new + assert!( + normalized_sql.contains("OLD.x_new"), + "Trigger SQL should contain OLD.x_new: {normalized_sql}", + ); + // WHERE x should remain as x (not x_new) because it refers to table u's column + assert!( + normalized_sql.contains("WHERE x =") || normalized_sql.contains("WHERE x="), + "Trigger SQL should contain WHERE x = (not WHERE x_new =): {normalized_sql}", + ); + assert!( + !normalized_sql.contains("WHERE x_new =") && !normalized_sql.contains("WHERE x_new="), + "Trigger SQL should NOT contain WHERE x_new = (x refers to table u): {normalized_sql}", + ); +} + +#[test] +fn test_alter_table_rename_column_update_of_column_list_rewritten() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create table + conn.execute("CREATE TABLE t (x INTEGER, y INTEGER)") + .unwrap(); + conn.execute("CREATE TABLE u (x INTEGER, z INTEGER)") + .unwrap(); + + // Create trigger with UPDATE OF x + conn.execute( + "CREATE TRIGGER tu AFTER UPDATE OF x ON t BEGIN + UPDATE u SET z = NEW.x WHERE x = OLD.x; + END", + ) + .unwrap(); + + // Rename column x to x_new in table t + conn.execute("ALTER TABLE t RENAME COLUMN x TO x_new") + .unwrap(); + + // Verify trigger SQL: UPDATE OF x should become UPDATE OF x_new + 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"), + } + } + + let normalized_sql = trigger_sql.split_whitespace().collect::>().join(" "); + // UPDATE OF x should become UPDATE OF x_new + assert!( + normalized_sql.contains("UPDATE OF x_new"), + "Trigger SQL should contain UPDATE OF x_new: {normalized_sql}", + ); + assert!( + !normalized_sql.contains("UPDATE OF x,") && !normalized_sql.contains("UPDATE OF x "), + "Trigger SQL should NOT contain UPDATE OF x (x was renamed to x_new): {normalized_sql}", + ); +} + +#[test] +fn test_alter_table_rename_column_update_of_multiple_columns_rewritten() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create table + conn.execute("CREATE TABLE t (x INTEGER, y INTEGER)") + .unwrap(); + conn.execute("CREATE TABLE u (x INTEGER, z INTEGER)") + .unwrap(); + + // Create trigger with UPDATE OF x, y + conn.execute( + "CREATE TRIGGER tu AFTER UPDATE OF x, y ON t BEGIN + UPDATE u SET z = NEW.x WHERE x = OLD.x; + END", + ) + .unwrap(); + + // Rename column x to x_new in table t + conn.execute("ALTER TABLE t RENAME COLUMN x TO x_new") + .unwrap(); + + // Verify trigger SQL: UPDATE OF x, y should become UPDATE OF x_new, y + 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"), + } + } + + let normalized_sql = trigger_sql.split_whitespace().collect::>().join(" "); + // UPDATE OF x, y should become UPDATE OF x_new, y + assert!( + normalized_sql.contains("UPDATE OF x_new, y") + || normalized_sql.contains("UPDATE OF x_new,y"), + "Trigger SQL should contain UPDATE OF x_new, y: {normalized_sql}", + ); + assert!( + !normalized_sql.contains("UPDATE OF x,") && !normalized_sql.contains("UPDATE OF x "), + "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" + ); +} + +#[test] +fn test_alter_table_rename_column_qualified_reference_to_trigger_table() { + let db = TempDatabase::new_empty(); + let conn = db.connect_limbo(); + + // Create two tables + conn.execute("CREATE TABLE t (x INTEGER, y INTEGER)") + .unwrap(); + conn.execute("CREATE TABLE u (z INTEGER)").unwrap(); + + // Note: SQLite doesn't support qualified references to the trigger's owning table (t.x). + // SQLite fails the RENAME COLUMN operation with "error in trigger tu: no such column: t.x". + // We match SQLite's behavior - the rename should fail. + + // Create trigger on t that uses qualified reference t.x (invalid in SQLite) + conn.execute( + "CREATE TRIGGER tu AFTER UPDATE ON t BEGIN + UPDATE u SET z = t.x WHERE z = 1; + END", + ) + .unwrap(); + + // Rename column x to x_new in table t should fail (SQLite fails with "no such column: t.x") + let result = conn.execute("ALTER TABLE t RENAME COLUMN x TO x_new"); + assert!( + result.is_err(), + "RENAME COLUMN should fail when trigger uses qualified reference to trigger table" + ); + + 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 or column: {error_msg}", + ); +}