add spaces and fix tests

Pavan-Nambi · Pavan-Nambi · commit b28e19455652 · 2025-12-18T16:03:32.000+05:30
dont allow table refs in when

adjust spaces

adjust spaces
diff --git a/core/translate/alter.rs b/core/translate/alter.rs
@@ -2644,6 +2644,51 @@ pub fn rewrite_expr_table_refs_for_rename(
     changed
 }
 
+/// Check if a trigger WHEN clause contains table-qualified column refs that would become
+/// invalid after a table rename. SQLite rejects ALTER TABLE RENAME if the WHEN clause
+/// contains refs like `t1.col` (other than NEW.col or OLD.col) where t1 is the table
+/// being renamed.
+pub fn check_when_clause_for_invalid_table_refs(
+    expr: &ast::Expr,
+    table_being_renamed: &str,
+) -> Option<(String, String)> {
+    let mut invalid_ref: Option<(String, String)> = None;
+
+    let _ = walk_expr(expr, &mut |e: &ast::Expr| -> Result<WalkControl> {
+        // If we already found an invalid ref, skip the rest
+        if invalid_ref.is_some() {
+            return Ok(WalkControl::SkipChildren);
+        }
+        match e {
+            ast::Expr::Qualified(tbl_name, col_name) => {
+                let tbl_norm = normalize_ident(tbl_name.as_str());
+                // NEW and OLD are valid pseudo-table references in triggers
+                if !tbl_norm.eq_ignore_ascii_case("new")
+                    && !tbl_norm.eq_ignore_ascii_case("old")
+                    && tbl_norm == table_being_renamed
+                {
+                    invalid_ref = Some((tbl_name.to_string(), col_name.to_string()));
+                    return Ok(WalkControl::SkipChildren);
+                }
+            }
+            ast::Expr::DoublyQualified(_, tbl_name, col_name) => {
+                let tbl_norm = normalize_ident(tbl_name.as_str());
+                if !tbl_norm.eq_ignore_ascii_case("new")
+                    && !tbl_norm.eq_ignore_ascii_case("old")
+                    && tbl_norm == table_being_renamed
+                {
+                    invalid_ref = Some((tbl_name.to_string(), col_name.to_string()));
+                    return Ok(WalkControl::SkipChildren);
+                }
+            }
+            _ => {}
+        }
+        Ok(WalkControl::Continue)
+    });
+
+    invalid_ref
+}
+
 /// Rewrite table references in a SELECT statement for table rename.
 ///
 /// Handles the complete SELECT structure used in trigger commands:
diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs
@@ -14,8 +14,8 @@ use crate::storage::page_cache::PageCache;
 use crate::storage::pager::{default_page1, CreateBTreeFlags};
 use crate::storage::sqlite3_ondisk::{read_varint_fast, DatabaseHeader, PageSize, RawVersion};
 use crate::translate::alter::{
-    rewrite_expr_table_refs_for_rename, rewrite_select_table_refs_for_rename,
-    walk_upsert_for_table_rename,
+    check_when_clause_for_invalid_table_refs, rewrite_expr_table_refs_for_rename,
+    rewrite_select_table_refs_for_rename, walk_upsert_for_table_rename,
 };
 use crate::translate::collate::CollationSeq;
 use crate::types::{
@@ -5856,6 +5856,17 @@ pub fn op_function(
                                 }
 
                                 if let Some(ref mut when_expr) = when_clause {
+                                    if let Some((tbl, col)) = check_when_clause_for_invalid_table_refs(
+                                        when_expr,
+                                        &rename_from,
+                                    ) {
+                                        return Err(LimboError::Constraint(format!(
+                                            "error in trigger {}: no such column: {}.{}",
+                                            trigger_name.name.as_str(),
+                                            tbl,
+                                            col
+                                        )));
+                                    }
                                     changed |= rewrite_expr_table_refs_for_rename(
                                         when_expr,
                                         &rename_from,
diff --git a/parser/src/ast/fmt.rs b/parser/src/ast/fmt.rs
@@ -398,9 +398,11 @@ impl ToTokens for Stmt {
                 }
                 s.append(TK_BEGIN, Some("\n"))?;
                 for command in commands {
+                    s.append(TK_ANY, Some("    "))?;
                     command.to_tokens(s, context)?;
                     s.append(TK_SEMI, Some("\n"))?;
                 }
+                s.append(TK_ANY, Some("  "))?;
                 s.append(TK_END, None)
             }
             Self::CreateView {
diff --git a/testing/trigger.test b/testing/trigger.test
@@ -825,43 +825,52 @@ do_execsql_test_on_specific_db {:memory:} trigger-rename-table-on-clause {
   ALTER TABLE t1 RENAME TO t1_new;
   SELECT sql FROM sqlite_schema WHERE type='trigger' AND name='trg1';
 } {{CREATE TRIGGER trg1 AFTER INSERT ON "t1_new" BEGIN
-INSERT INTO "t1_new" VALUES (100, 'triggered');
-END}}
+    INSERT INTO "t1_new" VALUES (100, 'triggered');
+  END}}
 
-# table-qualified refs in WHERE clause get rewritten
-do_execsql_test_on_specific_db {:memory:} trigger-rename-table-qualified-refs {
+# table-qualified refs in WHEN clause cause error on rename (SQLite behavior)
+do_execsql_test_in_memory_any_error trigger-rename-table-qualified-refs-when-error {
   CREATE TABLE t1 (id INT, val TEXT);
   CREATE TRIGGER trg2 AFTER UPDATE ON t1 WHEN t1.id > 5 BEGIN
     UPDATE t1 SET val = 'updated' WHERE t1.id = NEW.id;
   END;
   ALTER TABLE t1 RENAME TO t1_new;
+}
+
+# table-qualified refs in trigger body (not WHEN) get rewritten
+do_execsql_test_on_specific_db {:memory:} trigger-rename-table-qualified-refs-body {
+  CREATE TABLE t1 (id INT, val TEXT);
+  CREATE TRIGGER trg2 AFTER UPDATE ON t1 BEGIN
+    UPDATE t1 SET val = 'updated' WHERE t1.id = NEW.id;
+  END;
+  ALTER TABLE t1 RENAME TO t1_new;
   SELECT sql FROM sqlite_schema WHERE type='trigger' AND name='trg2';
-} {{CREATE TRIGGER trg2 AFTER UPDATE ON "t1_new" WHEN "t1_new".id > 5 BEGIN
-UPDATE "t1_new" SET val = 'updated' WHERE "t1_new".id = NEW.id;
-END}}
+} {{CREATE TRIGGER trg2 AFTER UPDATE ON "t1_new" BEGIN
+    UPDATE "t1_new" SET val = 'updated' WHERE "t1_new".id = NEW.id;
+  END}}
 
 do_execsql_test_on_specific_db {:memory:} trigger-rename-upsert-clause {
   CREATE TABLE t1 (id INT PRIMARY KEY, val TEXT);
   CREATE TRIGGER trg3 AFTER INSERT ON t1 BEGIN
-    INSERT INTO t1 VALUES (100, 'x') ON CONFLICT(id) DO UPDATE SET val = t1.val || 'conflict';
+    INSERT INTO t1 VALUES (100, 'x') ON CONFLICT (id) DO UPDATE SET val = t1.val || 'conflict';
   END;
   ALTER TABLE t1 RENAME TO t1_new;
   SELECT sql FROM sqlite_schema WHERE type='trigger' AND name='trg3';
 } {{CREATE TRIGGER trg3 AFTER INSERT ON "t1_new" BEGIN
-INSERT INTO "t1_new" VALUES (100, 'x') ON CONFLICT (id) DO UPDATE SET val = "t1_new".val || 'conflict';
-END}}
+    INSERT INTO "t1_new" VALUES (100, 'x') ON CONFLICT (id) DO UPDATE SET val = "t1_new".val || 'conflict';
+  END}}
 
 do_execsql_test_on_specific_db {:memory:} trigger-rename-insert-select-body {
   CREATE TABLE t1 (id INT, val TEXT);
   CREATE TABLE log (src TEXT, cnt INT);
   CREATE TRIGGER trg4 AFTER INSERT ON t1 BEGIN
-    INSERT INTO log SELECT 'inserted', count(*) FROM t1;
+    INSERT INTO log SELECT 'inserted', count (*) FROM t1;
   END;
   ALTER TABLE t1 RENAME TO t1_new;
   SELECT sql FROM sqlite_schema WHERE type='trigger' AND name='trg4';
 } {{CREATE TRIGGER trg4 AFTER INSERT ON "t1_new" BEGIN
-INSERT INTO log SELECT 'inserted', count (*) FROM "t1_new";
-END}}
+    INSERT INTO log SELECT 'inserted', count (*) FROM "t1_new";
+  END}}
 
 do_execsql_test_on_specific_db {:memory:} trigger-rename-select-cmd {
   CREATE TABLE t1 (id INT);
@@ -871,8 +880,8 @@ do_execsql_test_on_specific_db {:memory:} trigger-rename-select-cmd {
   ALTER TABLE t1 RENAME TO t1_new;
   SELECT sql FROM sqlite_schema WHERE type='trigger' AND name='trg5';
 } {{CREATE TRIGGER trg5 AFTER INSERT ON "t1_new" BEGIN
-SELECT count (*) FROM "t1_new" WHERE "t1_new".id > 0;
-END}}
+    SELECT count (*) FROM "t1_new" WHERE "t1_new".id > 0;
+  END}}
 
 # SQLite disallows RETURNING clause inside triggers
 do_execsql_test_in_memory_any_error trigger-returning-not-allowed {
@@ -916,3 +925,16 @@ do_execsql_test_on_specific_db {:memory:} trigger-fires-after-table-rename {
   SELECT count(*) FROM t2;
 } {2
 4}
+
+# table-qualified refs in WHEN clause subquery get rewritten (SQLite behavior)
+do_execsql_test_on_specific_db {:memory:} trigger-rename-table-qualified-refs-when-subquery {
+  CREATE TABLE t1 (id INT, val TEXT);
+  CREATE TRIGGER trg2 AFTER UPDATE ON t1 WHEN (SELECT id FROM t1 WHERE t1.id = NEW.id) > 5 BEGIN
+    SELECT 1;
+  END;
+  ALTER TABLE t1 RENAME TO t1_new;
+  SELECT sql FROM sqlite_schema WHERE type='trigger' AND name='trg2';
+} {{CREATE TRIGGER trg2 AFTER UPDATE ON "t1_new" WHEN (SELECT id FROM "t1_new" WHERE "t1_new".id = NEW.id) > 5 BEGIN
+    SELECT 1;
+  END}}
+

Original file line number	Diff line number	Diff line change
`@@ -398,9 +398,11 @@ impl ToTokens for Stmt {`
`398`	`398`	`}`
`399`	`399`	`s.append(TK_BEGIN, Some("\n"))?;`
`400`	`400`	`for command in commands {`
	`401`	`+ s.append(TK_ANY, Some(" "))?;`
`401`	`402`	`command.to_tokens(s, context)?;`
`402`	`403`	`s.append(TK_SEMI, Some("\n"))?;`
`403`	`404`	`}`
	`405`	`+ s.append(TK_ANY, Some(" "))?;`
`404`	`406`	`s.append(TK_END, None)`
`405`	`407`	`}`
`406`	`408`	`Self::CreateView {`