feat: batch chained DDL statements into a single migration (#11038)

* feat: batch chained DDL statements into a single migration prompt Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N8jvLAnZGMDQTm3Q8WQ3u6 * fix: keep a statement terminator out of a trailing sql line comment Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N8jvLAnZGMDQTm3Q8WQ3u6 * docs: record why a ddl run is grouped without proving it transaction-safe Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N8jvLAnZGMDQTm3Q8WQ3u6 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-10 08:07:03 +00:00 · 2026-09-09 11:46:24 +02:00
parent 1076b638d9
commit 656e609595
5 changed files with 120 additions and 33 deletions
@@ -3,7 +3,7 @@
 	import Modal2 from './common/modal/Modal2.svelte'
 	import NewDataTableMigrationModal from './workspaceSettings/NewDataTableMigrationModal.svelte'
 	import DataTableMigrationsButton from './workspaceSettings/DataTableMigrationsButton.svelte'
-	import { splitSqlStatements, isDdlStatement } from './sqlDdl'
+	import { joinSqlStatements, splitSqlRuns } from './sqlDdl'
 	import { logDdlGuardChoice } from './workspaceSettings/datatableTelemetry'
 	import { CornerDownLeft } from 'lucide-svelte'

@@ -11,7 +11,7 @@

 	type Choice = 'run' | 'migrate' | 'cancel'

-	let promptStatement = $state<string | undefined>(undefined)
+	let promptStatements = $state<string[]>([])
 	let promptOpen = $state(false)
 	let resolvePrompt: ((choice: Choice) => void) | undefined = undefined
 	let resolveMigrationClosed: ((created: boolean) => void) | undefined = undefined
@@ -23,11 +23,15 @@
 	// toast action after a migration is created here.
 	let migrationsModal = $state<DataTableMigrationsButton | undefined>(undefined)

+	// The block is shown as-is in the prompt and becomes the migration body, where
+	// every statement inside the BEGIN; ... END; frame must be `;`-terminated.
+	let promptSql = $derived(joinSqlStatements(promptStatements))
+
 	function finishPrompt(choice: Choice) {
 		const r = resolvePrompt
 		resolvePrompt = undefined
 		promptOpen = false
-		promptStatement = undefined
+		promptStatements = []
 		r?.(choice)
 	}

@@ -49,10 +53,10 @@
 		}
 	}

-	function promptDdl(statement: string): Promise<Choice> {
+	function promptDdl(statements: string[]): Promise<Choice> {
 		return new Promise((resolve) => {
 			resolvePrompt = resolve
-			promptStatement = statement
+			promptStatements = statements
 			promptOpen = true
 		})
 	}
@@ -66,49 +70,51 @@

 	// Open the prefilled new-migration modal. Resolves with whether a migration
 	// was actually created (false if the user cancelled / closed it).
-	function openMigrationModal(statement: string): Promise<boolean> {
+	function openMigrationModal(sql: string): Promise<boolean> {
 		return new Promise((resolve) => {
 			resolveMigrationClosed = (created: boolean) => resolve(created)
-			newMigrationModal?.open({ codeUp: statement })
+			newMigrationModal?.open({ codeUp: sql })
 		})
 	}

 	/**
-	 * Inspect `code` for DDL statements. For each one, prompt the user to run it
-	 * anyway or turn it into a migration (prompts shown one at a time). Returns
-	 * whether to proceed and the code to run (with migrated statements stripped).
+	 * Inspect `code` for DDL statements. Each run of adjacent DDL statements is
+	 * prompted for once (runs shown one at a time) and becomes a single migration,
+	 * so a chain of schema changes applies in one transaction instead of asking
+	 * once per statement. Returns whether to proceed and the code to run (with
+	 * migrated statements stripped).
 	 */
 	export async function guard(
 		code: string
 	): Promise<{ proceed: boolean; code: string; ranMigration: boolean }> {
 		migrationRan = false
-		const statements = splitSqlStatements(code)
-		if (!statements.some((s) => isDdlStatement(s))) {
+		const runs = splitSqlRuns(code)
+		if (!runs.some((r) => r.isDdl)) {
 			return { proceed: true, code, ranMigration: false }
 		}

 		const kept: string[] = []
-		for (const statement of statements) {
-			if (!isDdlStatement(statement)) {
-				kept.push(statement)
+		for (const run of runs) {
+			if (!run.isDdl) {
+				kept.push(...run.statements)
 				continue
 			}
-			// Re-prompt for this statement until the user makes a terminal choice;
+			// Re-prompt for this run until the user makes a terminal choice;
 			// cancelling the migration modal returns to the prompt with the DDL intact.
 			for (;;) {
-				const choice = await promptDdl(statement)
+				const choice = await promptDdl(run.statements)
 				if (choice === 'cancel') {
 					logDdlGuardChoice('cancelled')
 					return { proceed: false, code, ranMigration: migrationRan }
 				}
 				if (choice === 'run') {
 					logDdlGuardChoice('run_anyway')
-					kept.push(statement)
+					kept.push(...run.statements)
 					break
 				}
-				// migrate: only strip the statement once a migration is actually
+				// migrate: only strip the statements once a migration is actually
 				// created; if the modal was cancelled, loop back to the prompt.
-				const created = await openMigrationModal(statement)
+				const created = await openMigrationModal(joinSqlStatements(run.statements))
 				if (created) {
 					logDdlGuardChoice('migrated')
 					break
@@ -116,14 +122,14 @@
 			}
 		}

-		return { proceed: true, code: kept.join(';\n'), ranMigration: migrationRan }
+		return { proceed: true, code: joinSqlStatements(kept), ranMigration: migrationRan }
 	}
 </script>

 <svelte:window onkeydown={onKeyDown} />

 <Modal2
-	title="Schema change detected"
+	title={promptStatements.length > 1 ? 'Schema changes detected' : 'Schema change detected'}
 	fixedWidth="md"
 	fixedHeight="adaptive"
 	bind:isOpen={promptOpen}
@@ -131,12 +137,17 @@
 >
 	<div class="flex flex-col gap-3 w-full">
 		<p class="text-sm text-secondary">
-			This looks like a schema-changing (DDL) statement. Schema changes are best tracked as
-			migrations rather than run ad-hoc. Create a migration for it instead?
+			{#if promptStatements.length > 1}
+				These {promptStatements.length} consecutive statements are schema-changing (DDL). Schema changes
+				are best tracked as migrations rather than run ad-hoc. Create a single migration for them instead?
+			{:else}
+				This looks like a schema-changing (DDL) statement. Schema changes are best tracked as
+				migrations rather than run ad-hoc. Create a migration for it instead?
+			{/if}
 		</p>
 		<pre
 			class="text-xs whitespace-pre-wrap font-mono bg-surface-secondary rounded p-3 max-h-48 overflow-auto"
-			>{promptStatement ?? ''}</pre
+			>{promptSql}</pre
 		>
 		<div class="flex justify-end gap-2 pt-2">
 			<Button variant="default" size="sm" on:click={() => finishPrompt('run')}>Run anyway</Button>
@@ -1,5 +1,11 @@
 import { describe, expect, it } from 'vitest'
-import { endsWithUnterminatedStatement, splitSqlStatements, stripSqlComments } from './sqlDdl'
+import {
+	endsWithUnterminatedStatement,
+	joinSqlStatements,
+	splitSqlRuns,
+	splitSqlStatements,
+	stripSqlComments
+} from './sqlDdl'

 describe('endsWithUnterminatedStatement', () => {
 	it('reports a missing terminator on the final statement', () => {
@@ -67,3 +73,36 @@ describe('splitSqlStatements', () => {
 		])
 	})
 })
+
+describe('splitSqlRuns', () => {
+	it('groups adjacent DDL statements and breaks the run on a non-DDL one', () => {
+		expect(
+			splitSqlRuns(
+				'CREATE TABLE a (id int); ALTER TABLE a ADD b int; INSERT INTO a VALUES (1); DROP TABLE a;'
+			)
+		).toEqual([
+			{ isDdl: true, statements: ['CREATE TABLE a (id int)', 'ALTER TABLE a ADD b int'] },
+			{ isDdl: false, statements: ['INSERT INTO a VALUES (1)'] },
+			{ isDdl: true, statements: ['DROP TABLE a'] }
+		])
+	})
+
+	it('keeps a leading comment with the statement it introduces', () => {
+		expect(splitSqlRuns('-- add it\nCREATE TABLE a (id int);')).toEqual([
+			{ isDdl: true, statements: ['-- add it\nCREATE TABLE a (id int)'] }
+		])
+	})
+})
+
+describe('joinSqlStatements', () => {
+	it('keeps a terminator out of a statement-trailing line comment', () => {
+		expect(joinSqlStatements(['CREATE TABLE a (id int)', 'ALTER TABLE a ADD b int'])).toBe(
+			'CREATE TABLE a (id int);\nALTER TABLE a ADD b int;'
+		)
+		// A `;` appended after the comment would be commented out, gluing this
+		// statement onto the next one.
+		expect(joinSqlStatements(['CREATE TABLE a (id int) -- new', 'ALTER TABLE a ADD b int'])).toBe(
+			'CREATE TABLE a (id int) -- new\n;\nALTER TABLE a ADD b int;'
+		)
+	})
+})
@@ -258,3 +258,40 @@ export function isDdlStatement(statement: string): boolean {
 	const firstWord = pruneComments(statement).trim().split(/\s+/)[0]?.toUpperCase()
 	return !!firstWord && DDL_KEYWORDS.includes(firstWord)
 }
+
+export type SqlRun = { isDdl: boolean; statements: string[] }
+
+/**
+ * Join statements into one runnable block, `;`-terminating each. `splitSqlStatements`
+ * drops the `;` it splits on and keeps a statement's trailing comment, so a statement
+ * ending in a `--` comment would swallow a terminator appended on the same line and
+ * run into the next statement; that one gets its terminator on its own line.
+ */
+export function joinSqlStatements(statements: string[], backslashEscapes = true): string {
+	return statements
+		.map((s) => (endsWithUnterminatedStatement(`${s};`, backslashEscapes) ? `${s}\n;` : `${s};`))
+		.join('\n')
+}
+
+/**
+ * Split a script into alternating runs of DDL and non-DDL statements. Adjacent
+ * DDL statements share a run so they can become one migration instead of one
+ * prompt each; a non-DDL statement ends the run, since anything after it may
+ * depend on it and the surviving statements must keep their relative order.
+ *
+ * A run is deliberately not proven safe to run in one transaction — Postgres refuses
+ * to use an enum value in the same transaction that `ALTER TYPE` added it, and there
+ * are other such pairs. Grouping them anyway is the accepted trade: the body is shown
+ * in the migration editor before it runs, and a failed run is reverted with the
+ * database's own error, so splitting it by hand is a visible one-step fix.
+ */
+export function splitSqlRuns(code: string, backslashEscapes = true): SqlRun[] {
+	const runs: SqlRun[] = []
+	for (const statement of splitSqlStatements(code, backslashEscapes)) {
+		const isDdl = isDdlStatement(statement)
+		const last = runs[runs.length - 1]
+		if (last && last.isDdl === isDdl) last.statements.push(statement)
+		else runs.push({ isDdl, statements: [statement] })
+	}
+	return runs
+}
@@ -64,9 +64,8 @@
 	function wrapInTransaction(body: string): string {
 		return `BEGIN;\n\n${body}\n\nEND;`
 	}
-	// A statement inside BEGIN; ... END; must be `;`-terminated. The SQL splitter
-	// strips the trailing `;` when extracting a detected DDL statement, so re-add
-	// it when missing.
+	// Every statement inside BEGIN; ... END; must be `;`-terminated, so normalize
+	// a prefilled body that ends without one.
 	function ensureTrailingSemicolon(body: string): string {
 		const trimmed = body.trimEnd()
 		return trimmed.endsWith(';') ? trimmed : `${trimmed};`
@@ -28,13 +28,14 @@ export type DdlGuardChoice =
 	| 'run_anyway'
 	/** The DDL became a migration definition. */
 	| 'migrated'
-	/** The statement was abandoned, so nothing ran. */
+	/** The statements were abandoned, so nothing ran. */
 	| 'cancelled'

 /**
- * Counted once per prompt that reaches a terminal choice. Picking "create a migration" and then
- * dismissing the modal loops back to the prompt instead, and is deliberately not counted: it is
- * the same statement still undecided, not a fourth outcome.
+ * Counted once per prompt that reaches a terminal choice — a prompt covers a whole run of
+ * adjacent DDL statements, not one statement. Picking "create a migration" and then dismissing
+ * the modal loops back to the prompt instead, and is deliberately not counted: it is the same
+ * run still undecided, not a fourth outcome.
 */
 export function logDdlGuardChoice(choice: DdlGuardChoice): void {
 	logFeatureUsage('datatable', 'ddl_guard', { key: choice })