fix(sql): keep trigger bodies in one statement, and terminate them

packages/sql/src/modules/migrations/sql-generator.module.test.ts

@@ -1626,3 +1626,43 @@ describe('SQLite views: no CREATE OR REPLACE, no doubled header', () => {
     expect(sql).toMatch(/CREATE OR REPLACE VIEW v_customers AS/i);
   });
 });
+
+describe('trigger definitions are terminated', () => {
+  // SQLite's sqlite_master.sql ends a trigger at `END`, with no `;`. Emitted as
+  // it stands, the CREATE TRIGGER never ends and swallows the next statement.
+  const body = `CREATE TRIGGER trg_decrement_stock
+AFTER INSERT ON order_items
+FOR EACH ROW
+BEGIN
+  UPDATE products SET stock = stock - NEW.qty WHERE id = NEW.product_id;
+END`;
+
+  const withTrigger = (): TableDiff => ({
+    tableName: 'ORDER_ITEMS',
+    objectType: 'TABLE',
+    status: 'ADDED',
+    columnDiffs: [],
+    indexDiffs: [],
+    foreignKeyDiffs: [],
+    sourceTable: tableSchema({
+      name: 'order_items',
+      columns: [{ name: 'qty', type: 'INTEGER', nullable: false, primaryKey: false }],
+      triggers: [{ name: 'trg_decrement_stock', timing: 'AFTER', event: 'INSERT', definition: body }],
+    }),
+  });
+
+  it('terminates a catalog definition that ends at END', () => {
+    const sql = gen.generateMigrationSql([withTrigger()], 'sqlite');
+    expect(sql).toMatch(/END;/);
+    // nothing may follow END on the same statement without a terminator
+    expect(sql).not.toMatch(/END[ \t]*\n[ \t]*\n?[ \t]*(ALTER|CREATE|DROP)\b/i);
+  });
+
+  it('does not double the terminator when one is already there', () => {
+    const diff = withTrigger();
+    diff.sourceTable!.triggers![0]!.definition = `${body};`;
+    const sql = gen.generateMigrationSql([diff], 'sqlite');
+    expect(sql).toMatch(/END;/);
+    expect(sql).not.toMatch(/END;;/);
+  });
+});

packages/sql/src/modules/migrations/sql-generator.module.ts

@@ -480,8 +480,11 @@ export class SqlGeneratorModule {
     }
 
     for (const trg of table.triggers ?? []) {
+      // ensureSemicolon, not a bare trim: a catalog definition can end at `END`
+      // with no terminator (SQLite's sqlite_master.sql does), and an
+      // unterminated CREATE TRIGGER swallows whatever statement follows it.
       sql += trg.definition
-        ? `${trg.definition.trim()}\n`
+        ? `${this.ensureSemicolon(trg.definition)}\n`
         : `-- Trigger ${trg.name} (${trg.timing ?? ''} ${trg.event ?? ''}) has no available definition\n`;
     }
 
@@ -591,7 +594,10 @@ export class SqlGeneratorModule {
       for (const trg of source.triggers ?? []) {
         if (!trg.definition) continue;
         const stmt = dialect.createTriggerStatement?.(trg, name) ?? null;
-        statements.push(stmt ?? trg.definition.trim());
+        // See renderCreateTable: an unterminated CREATE TRIGGER swallows the
+        // next statement. The dialect hooks already end in `;`; this is a no-op
+        // for them and supplies the terminator for a raw catalog definition.
+        statements.push(this.ensureSemicolon(stmt ?? trg.definition));
       }
     } else if (obj.objectType === 'SEQUENCE' && source) {
       statements.push(this.renderCreateSequence({ ...source, name }, dialect));
@@ -946,7 +952,7 @@ export class SqlGeneratorModule {
         if (!trg.source?.definition) continue;
         const trgSpec = { name: trg.source.name, timing: trg.source.timing, event: trg.source.event, definition: trg.source.definition };
         const stmt = dialect.createTriggerStatement?.(trgSpec, tableName) ?? null;
-        statements.push(stmt ?? trg.source.definition.trim());
+        statements.push(this.ensureSemicolon(stmt ?? trg.source.definition));
       }
 
       // Recreate any views dropped by the dialect's dropDependentViewsBlock above.

packages/sql/src/modules/sql-text/sql-splitter.test.ts

@@ -1109,3 +1109,47 @@ describe('splitting on the shared tokenizer', () => {
     expect(second).toMatchObject({ startLine: 3, endLine: 3 });
   });
 });
+
+describe('splitSqlStatements: a `.` in a comment does not qualify the next identifier', () => {
+  // Found by running a real SQLite migration. The generator writes review notes
+  // above the statements they concern, e.g.
+  //   -- review: skip FK … declare FOREIGN KEY in CREATE TABLE only (no ALTER ADD).
+  // Qualification used to be decided by scanning backwards over whitespace for a
+  // `.`, which cannot tell code from comment text. The note's trailing period made
+  // the following `CREATE` look like `schema.CREATE`, so it was skipped, the
+  // statement never registered as routine DDL, and the trigger body split at the
+  // `;` inside BEGIN … END — leaving a fragment starting with `END`.
+  const trigger = `CREATE TRIGGER trg_decrement_stock
+AFTER INSERT ON order_items
+FOR EACH ROW
+BEGIN
+  UPDATE products SET stock = stock - NEW.qty WHERE id = NEW.product_id;
+END;`;
+
+  it('keeps a trigger whole after a comment ending in a period', () => {
+    const sql = `-- review: declare FOREIGN KEY in CREATE TABLE only (no ALTER ADD).\n${trigger}`;
+    const stmts = splitSqlStatements(sql);
+    expect(stmts).toHaveLength(1);
+    expect(stmts[0]!.text).toContain('UPDATE products');
+    expect(stmts.some((s) => /^\s*END\b/i.test(s.text))).toBe(false);
+  });
+
+  it.each(['--.', '-- .', '--a.', '-- ends with a period.'])(
+    'keeps a trigger whole after the comment %j',
+    (comment) => {
+      expect(splitSqlStatements(`${comment}\n${trigger}`)).toHaveLength(1);
+    }
+  );
+
+  it('still treats a genuinely qualified identifier as qualified', () => {
+    // `s.begin` / `s.end` are column names, not block delimiters: if they were
+    // read as a BEGIN opener the `;` would stop splitting.
+    expect(splitSqlStatements('SELECT s.begin FROM s;\nSELECT 1;')).toHaveLength(2);
+    expect(splitSqlStatements('SELECT s.end FROM s;\nSELECT 1;')).toHaveLength(2);
+  });
+
+  it('reads a qualifier across whitespace and comments', () => {
+    expect(splitSqlStatements('SELECT s . begin FROM s;\nSELECT 1;')).toHaveLength(2);
+    expect(splitSqlStatements('SELECT s /* c */ . begin FROM s;\nSELECT 1;')).toHaveLength(2);
+  });
+});

packages/sql/src/modules/sql-text/sql-splitter.ts

@@ -361,12 +361,6 @@ const TRANSACTION_BEGIN = new Set([
 /** Block frames pushed while scanning routine DDL bodies. */
 type RoutineBlock = 'begin' | 'case' | 'try' | 'catch';
 
-function identIsQualified(sql: string, identStart: number): boolean {
-  let k = identStart - 1;
-  while (k >= 0 && /\s/.test(sql[k]!)) k--;
-  return k >= 0 && sql[k] === '.';
-}
-
 /** Split a SQL-only buffer into `;`-terminated statements (no code fences). */
 function splitSqlOnly(sql: string): SplitStatement[] {
   const out: SplitStatement[] = [];
@@ -396,6 +390,19 @@ function splitSqlOnly(sql: string): SplitStatement[] {
   let blockStack: RoutineBlock[] = [];
   /** True at the start of a procedural statement (`BEGIN` body / after `;`). */
   let atProcStmtStart = false;
+  /**
+   * The previous code token was `.`, so the identifier after it is qualified —
+   * `s.begin` is a column name, not a block opener.
+   *
+   * Read from the token stream rather than by scanning the raw text backwards.
+   * A backward scan that skips only whitespace cannot tell a `.` that is code
+   * from one that merely ends a comment, so a review note like
+   * `-- … in CREATE TABLE only (no ALTER ADD).` sitting above a trigger made
+   * its `CREATE` look qualified. `CREATE` was then skipped, the statement never
+   * registered as routine DDL, and the trigger body split at the first `;`
+   * inside `BEGIN … END`.
+   */
+  let prevTokenWasDot = false;
   /** Swallow the next ident after `END IF` / `END CASE` so it is not an opener. */
   let skipNextWord: string | null = null;
 
@@ -452,7 +459,7 @@ function splitSqlOnly(sql: string): SplitStatement[] {
   const onWord = (wordStart: number, wordEnd: number) => {
     const raw = sql.slice(wordStart, wordEnd).toUpperCase();
     const word = raw === 'PROC' ? 'PROCEDURE' : raw;
-    if (identIsQualified(sql, wordStart)) {
+    if (prevTokenWasDot) {
       if (inRoutineDdl) atProcStmtStart = false;
       return;
     }
@@ -546,6 +553,10 @@ function splitSqlOnly(sql: string): SplitStatement[] {
     } else if (token.kind === 'word' && /[A-Za-z_]/.test(sql[token.start]!)) {
       onWord(token.start, token.end);
     }
+    // After onWord, so it describes the token before the next identifier.
+    // Comments and whitespace do not clear it: `a . b` and `a /* c */ . b` are
+    // both qualified, while a `.` that only ends a comment never gets here.
+    if (!isTrivia(token)) prevTokenWasDot = token.kind === 'punct' && token.upper === '.';
     for (let k = token.start; k < token.end; k++) if (sql.charCodeAt(k) === 10) line++;
   }
   flush(len, false);