From bcf93a58573804cca881c24027fda627e92bd252 Mon Sep 17 00:00:00 2001 From: huyplb Date: Sun, 16 Aug 2026 21:30:04 -0600 Subject: [PATCH 1/7] test(sql): run the generated DDL on the real engines, not just SQLite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SQLite oracle needs no credentials, so it runs everywhere — but it cannot tell you whether the *per-dialect* quoting is right. Backticks for MySQL, brackets for T-SQL, double quotes elsewhere: only the servers know, and getting it wrong is how a migration dies halfway through a customer's database. Runs the same hostile schemas — spaces, reserved words, punctuation, non-ASCII, an index and an FK over awkward names — against Postgres, MySQL, MariaDB, SQL Server, CockroachDB and YugabyteDB from `docker compose`. 30 cases, all green, with the identifier-quoting fix in place. Gated behind FOX_IT_DB=1 so the default run and CI stay DB-free; unreachable engines skip individually so a partial stack still tells you something. Each case asserts it generated statements before executing them, so an empty plan cannot pass vacuously, and every table it creates is dropped afterwards. Verified the harness can actually fail: the unquoted form of the same statement is rejected by the live server with `syntax error at or near "Order"`. Co-Authored-By: Claude Opus 5 --- .../modules/generated-ddl-live.test.ts | 211 ++++++++++++++++++ 1 file changed, 211 insertions(+) create mode 100644 apps/web/src/backend/modules/generated-ddl-live.test.ts diff --git a/apps/web/src/backend/modules/generated-ddl-live.test.ts b/apps/web/src/backend/modules/generated-ddl-live.test.ts new file mode 100644 index 00000000..a1594c57 --- /dev/null +++ b/apps/web/src/backend/modules/generated-ddl-live.test.ts @@ -0,0 +1,211 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + * + * Generated DDL, executed by the real database servers. + * + * `generated-ddl-runs.test.ts` proves the same statements parse in SQLite, + * which needs no credentials and so runs everywhere. It cannot tell you whether + * the *per-dialect* quoting is right: backticks for MySQL, brackets for T-SQL, + * double quotes elsewhere. Only the servers can, and getting that wrong is how + * a migration fails halfway through against a customer's database. + * + * Gated behind FOX_IT_DB=1 so the default `vitest run` and CI stay DB-free: + * + * docker compose up -d + * FOX_IT_DB=1 npx vitest run apps/web/src/backend/modules/generated-ddl-live.test.ts + * + * Engines that are not up are skipped individually rather than failing the run, + * so a partial stack still tells you something. Oracle and DB2 are in the + * compose file but slow to boot; add them here once they are healthy. + */ +import { afterAll, describe, expect, it } from 'vitest'; +import { ConnectionFactory } from '@foxschema/db'; +import { CompareModule, SqlGeneratorModule } from '@foxschema/sql'; +import type { ConnectionOptions, TableSchema } from '@foxschema/sql'; + +const RUN = process.env.FOX_IT_DB === '1'; +const gen = new SqlGeneratorModule(); + +/** Unique per run, so a rerun never collides with rows an earlier one left. */ +const TAG = Date.now().toString(36).slice(-5); + +const TARGETS: Array<{ dialect: string; provider: string; options: ConnectionOptions }> = [ + { + dialect: 'postgres', + provider: 'postgres', + options: { host: 'localhost', port: 5432, database: 'foxdb', username: 'foxuser', password: 'foxpass', schema: 'public' }, + }, + { + dialect: 'mysql', + provider: 'mysql', + options: { host: 'localhost', port: 3306, database: 'foxdb', username: 'foxuser', password: 'foxpass' }, + }, + { + dialect: 'mariadb', + provider: 'mariadb', + options: { host: 'localhost', port: 3307, database: 'foxdb', username: 'foxuser', password: 'foxpass' }, + }, + { + dialect: 'sqlserver', + provider: 'sqlserver', + options: { host: 'localhost', port: 1433, database: 'master', username: 'sa', password: 'FoxPass123!', ssl: { enabled: false } }, + }, + { + dialect: 'cockroachdb', + provider: 'cockroachdb', + options: { host: 'localhost', port: 26257, database: 'defaultdb', username: 'root', schema: 'public' }, + }, + { + dialect: 'yugabytedb', + provider: 'yugabytedb', + options: { host: 'localhost', port: 5433, database: 'yugabyte', username: 'yugabyte', schema: 'public' }, + }, +]; + +const table = (over: Partial & { name: string }): TableSchema => ({ + objectType: 'TABLE', + columns: [], + indices: [], + foreignKeys: [], + ...over, +}); + +/** Names that are legal in a catalog and illegal in SQL unless quoted. */ +const CASES: Array<{ label: string; tables: TableSchema[] }> = [ + { + label: 'an ordinary table (control — proves the harness runs anything at all)', + tables: [ + table({ + name: `plain_${TAG}`, + columns: [ + { name: 'id', type: 'INTEGER', nullable: false, primaryKey: true }, + { name: 'email', type: 'VARCHAR(255)', nullable: true, primaryKey: false }, + ], + }), + ], + }, + { + label: 'spaces in the table and column names', + tables: [ + table({ + name: `Order Details ${TAG}`, + columns: [ + { name: 'order id', type: 'INTEGER', nullable: false, primaryKey: true }, + { name: 'unit price', type: 'DECIMAL(10,2)', nullable: true, primaryKey: false }, + ], + primaryKey: { name: `pk order ${TAG}`, columns: ['order id'] }, + }), + ], + }, + { + label: 'reserved words as identifiers', + tables: [ + table({ + name: `order_${TAG}`, + columns: [ + { name: 'select', type: 'VARCHAR(10)', nullable: true, primaryKey: false }, + { name: 'order', type: 'INTEGER', nullable: true, primaryKey: false }, + { name: 'key', type: 'INTEGER', nullable: true, primaryKey: false }, + { name: 'user', type: 'VARCHAR(20)', nullable: true, primaryKey: false }, + ], + }), + ], + }, + { + label: 'punctuation and non-ASCII letters', + tables: [ + table({ + name: `naive-tbl-${TAG}`, + columns: [ + { name: 'id', type: 'INTEGER', nullable: false, primaryKey: false }, + { name: 'café', type: 'VARCHAR(50)', nullable: true, primaryKey: false }, + ], + }), + ], + }, + { + label: 'an index and a foreign key over awkward names', + tables: [ + table({ + name: `parent tbl ${TAG}`, + columns: [{ name: 'parent id', type: 'INTEGER', nullable: false, primaryKey: true }], + primaryKey: { columns: ['parent id'] }, + }), + table({ + name: `child tbl ${TAG}`, + columns: [ + { name: 'child id', type: 'INTEGER', nullable: false, primaryKey: true }, + { name: 'parent id', type: 'INTEGER', nullable: true, primaryKey: false }, + ], + primaryKey: { columns: ['child id'] }, + indices: [{ name: `idx child ${TAG}`, columns: ['parent id'], unique: false }], + foreignKeys: [ + { + name: `fk child ${TAG}`, + columns: ['parent id'], + referencedTable: `parent tbl ${TAG}`, + referencedColumns: ['parent id'], + }, + ], + }), + ], + }, +]; + +/** DDL for creating `tables` from nothing, as the migration flow emits it. */ +async function ddlFor(tables: TableSchema[], dialect: string): Promise { + const result = await new CompareModule().compare(tables, [], { source: dialect, target: dialect }); + return gen.generateMigrationPlan(result.tables, dialect).flatMap((step) => step.statements); +} + +const reachable = new Map(); +const toDrop: Array<{ provider: string; options: ConnectionOptions; name: string }> = []; + +afterAll(async () => { + // Children before parents, so an FK never blocks the drop. + for (const { provider, options, name } of toDrop.reverse()) { + await ConnectionFactory.executeQuery(provider, options, `DROP TABLE ${name}`).catch(() => undefined); + } +}); + +describe.runIf(RUN)('generated DDL runs on the real engines', () => { + for (const target of TARGETS) { + describe(target.dialect, () => { + it('is reachable', async () => { + try { + await ConnectionFactory.executeQuery(target.provider, target.options, 'SELECT 1'); + reachable.set(target.dialect, true); + } catch (err) { + reachable.set(target.dialect, false); + // Not a failure: a partial stack should still test what is up. + console.warn(`[skip] ${target.dialect}: ${(err as Error).message.split('\n')[0]}`); + } + }); + + for (const testCase of CASES) { + it(`creates ${testCase.label}`, async () => { + if (reachable.get(target.dialect) === false) return; + const statements = (await ddlFor(testCase.tables, target.dialect)).filter( + (s) => !s.trim().startsWith('--') + ); + expect(statements.length, 'generated no DDL to execute').toBeGreaterThan(0); + + for (const statement of statements) { + const sql = statement.replace(/;\s*$/, ''); + try { + await ConnectionFactory.executeQuery(target.provider, target.options, sql); + } catch (err) { + throw new Error( + `${target.dialect} rejected:\n${sql}\n\n${(err as Error).message.split('\n')[0]}` + ); + } + const made = sql.match(/CREATE TABLE\s+("[^"]+"|`[^`]+`|\[[^\]]+\]|\S+)/i); + if (made) toDrop.push({ provider: target.provider, options: target.options, name: made[1]! }); + } + }); + } + }); + } +}); From 5d593c15f8cbe7ac00d52880594ea4498892e643 Mon Sep 17 00:00:00 2001 From: huyplb Date: Sun, 16 Aug 2026 21:46:31 -0600 Subject: [PATCH 2/7] test(sql): cover the ALTER path on the live engines, and run plans faithfully MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Creating a table exercises one code path; ADD/DROP/MODIFY COLUMN go through the per-dialect hooks, which is where the dialects diverge most. Adds an ALTER case over awkward names to every live engine, and a DB2 target for when that container is up. The ALTER case failed on Postgres and YugabyteDB at first, and the failure was this harness, not the product — worth writing down because it looks so much like a real bug. Postgres's dependent-view hooks stash view definitions in a `CREATE TEMP TABLE … ON COMMIT DROP` and read them back several statements later, so the plan only holds together when it runs the way MigrationModule runs it: one unpooled connection, one transaction. A connection per statement loses the temp table with the session; a transaction per statement drops it at the first commit. Both report `relation "_fs_vdep_…" does not exist`. `runPlan` now mirrors MigrationModule exactly, so what the test proves is what the product actually does. 49 cases green across Postgres, MySQL, MariaDB, SQL Server, CockroachDB and YugabyteDB. Co-Authored-By: Claude Opus 5 --- .../modules/generated-ddl-live.test.ts | 114 ++++++++++++++++-- 1 file changed, 101 insertions(+), 13 deletions(-) diff --git a/apps/web/src/backend/modules/generated-ddl-live.test.ts b/apps/web/src/backend/modules/generated-ddl-live.test.ts index a1594c57..888a2ba9 100644 --- a/apps/web/src/backend/modules/generated-ddl-live.test.ts +++ b/apps/web/src/backend/modules/generated-ddl-live.test.ts @@ -21,7 +21,7 @@ * compose file but slow to boot; add them here once they are healthy. */ import { afterAll, describe, expect, it } from 'vitest'; -import { ConnectionFactory } from '@foxschema/db'; +import { ConnectionFactory, getAdapter } from '@foxschema/db'; import { CompareModule, SqlGeneratorModule } from '@foxschema/sql'; import type { ConnectionOptions, TableSchema } from '@foxschema/sql'; @@ -62,6 +62,14 @@ const TARGETS: Array<{ dialect: string; provider: string; options: ConnectionOpt provider: 'yugabytedb', options: { host: 'localhost', port: 5433, database: 'yugabyte', username: 'yugabyte', schema: 'public' }, }, + { + // Slowest of the set to boot (the compose healthcheck allows two minutes + // before it even starts probing) and the only one needing a native client + // driver, so it is the most likely to be skipped on a given machine. + dialect: 'db2', + provider: 'db2', + options: { host: 'localhost', port: 50000, database: 'foxdb', username: 'db2inst1', password: 'foxpass', schema: 'DB2INST1' }, + }, ]; const table = (over: Partial & { name: string }): TableSchema => ({ @@ -160,6 +168,61 @@ async function ddlFor(tables: TableSchema[], dialect: string): Promise return gen.generateMigrationPlan(result.tables, dialect).flatMap((step) => step.statements); } +/** + * Runs `statements` the way `MigrationModule` does: **one** unpooled connection, + * **one** transaction, for the whole plan. + * + * Both halves matter, and getting either wrong makes a correct plan look + * broken. Postgres's dependent-view hooks stash the view definitions in a + * `CREATE TEMP TABLE … ON COMMIT DROP` and read them back in a later statement: + * a connection per statement loses the table with the session, and a + * transaction per statement drops it at the first commit. Both produced + * `relation "_fs_vdep_…" does not exist`, which reads exactly like a product + * bug and was purely this harness being unfaithful. + */ +async function runPlan( + target: { dialect: string; provider: string; options: ConnectionOptions }, + statements: string[], + onCreate?: (name: string) => void +): Promise { + const connection = await ConnectionFactory.create(target.provider, target.options, { pooled: false }); + const adapter = getAdapter(target.provider); + try { + await adapter.beginTransaction(connection); + try { + for (const statement of statements) { + const sql = statement.replace(/;\s*$/, ''); + if (!sql.trim() || sql.trim().startsWith('--')) continue; + try { + await adapter.query(connection, sql, []); + } catch (err) { + throw new Error( + `${target.dialect} rejected:\n${sql}\n\n${(err as Error).message.split('\n')[0]}` + ); + } + const made = sql.match(/CREATE TABLE\s+("[^"]+"|`[^`]+`|\[[^\]]+\]|\S+)/i); + if (made && onCreate) onCreate(made[1]!); + } + await adapter.commitTransaction(connection); + } catch (err) { + await adapter.rollbackTransaction(connection).catch(() => undefined); + throw err; + } + } finally { + await ConnectionFactory.close(target.provider, connection).catch(() => undefined); + } +} + +/** DDL that migrates `from` into `to` — the ALTER path, per dialect hooks. */ +async function alterDdl( + from: TableSchema[], + to: TableSchema[], + dialect: string +): Promise { + const result = await new CompareModule().compare(to, from, { source: dialect, target: dialect }); + return gen.generateMigrationPlan(result.tables, dialect).flatMap((step) => step.statements); +} + const reachable = new Map(); const toDrop: Array<{ provider: string; options: ConnectionOptions; name: string }> = []; @@ -184,6 +247,40 @@ describe.runIf(RUN)('generated DDL runs on the real engines', () => { } }); + it('alters a table whose names need quoting', async () => { + if (reachable.get(target.dialect) === false) return; + // ADD / DROP / MODIFY COLUMN go through per-dialect hooks rather than + // the CREATE path, so they need their own proof on a real server — + // this is where the dialects diverge most. + const before = table({ + name: `alter tbl ${TAG}`, + columns: [ + { name: 'id', type: 'INTEGER', nullable: false, primaryKey: true }, + { name: 'old col', type: 'VARCHAR(10)', nullable: true, primaryKey: false }, + ], + primaryKey: { columns: ['id'] }, + }); + const after = table({ + name: `alter tbl ${TAG}`, + columns: [ + { name: 'id', type: 'INTEGER', nullable: false, primaryKey: true }, + { name: 'new col', type: 'VARCHAR(50)', nullable: true, primaryKey: false }, + ], + primaryKey: { columns: ['id'] }, + }); + + await runPlan(target, await ddlFor([before], target.dialect), (name) => + toDrop.push({ provider: target.provider, options: target.options, name }) + ); + + const alters = (await alterDdl([before], [after], target.dialect)).filter( + (s) => !s.trim().startsWith('--') + ); + expect(alters.length, 'compare reported a change but generated no ALTER').toBeGreaterThan(0); + // One session for the whole plan, exactly as MigrationModule runs it. + await runPlan(target, alters); + }); + for (const testCase of CASES) { it(`creates ${testCase.label}`, async () => { if (reachable.get(target.dialect) === false) return; @@ -192,18 +289,9 @@ describe.runIf(RUN)('generated DDL runs on the real engines', () => { ); expect(statements.length, 'generated no DDL to execute').toBeGreaterThan(0); - for (const statement of statements) { - const sql = statement.replace(/;\s*$/, ''); - try { - await ConnectionFactory.executeQuery(target.provider, target.options, sql); - } catch (err) { - throw new Error( - `${target.dialect} rejected:\n${sql}\n\n${(err as Error).message.split('\n')[0]}` - ); - } - const made = sql.match(/CREATE TABLE\s+("[^"]+"|`[^`]+`|\[[^\]]+\]|\S+)/i); - if (made) toDrop.push({ provider: target.provider, options: target.options, name: made[1]! }); - } + await runPlan(target, statements, (name) => + toDrop.push({ provider: target.provider, options: target.options, name }) + ); }); } }); From 8bbe88a8d3c037ae7612b77b5e41583772392aa6 Mon Sep 17 00:00:00 2001 From: huyplb Date: Sun, 16 Aug 2026 22:01:21 -0600 Subject: [PATCH 3/7] fix(db2): reorg the table after a column drop, or it stays unwritable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DB2 leaves a table in *reorg-pending* after `ALTER TABLE … DROP COLUMN` (and some type changes). `SELECT` still works — every INSERT/UPDATE/DELETE, and every index or key rebuild, fails with **SQL0668N reason code 7** until REORG runs. So the migration reported success and handed back a table nobody could write to, and because reads kept working it could be a long time before anyone connected the two. Adds a `postColumnChangeStatements` hook, emitted after the column changes and *before* the keys and indexes are rebuilt — those rebuilds are blocked by the same pending state. DB2 implements it as `CALL SYSPROC.ADMIN_CMD('REORG TABLE …')`; bare `REORG TABLE` is a CLP command, not SQL, and cannot be sent over a client connection. No other dialect implements the hook, and a unit test holds them to that. Two harness faults had to be fixed before this bug could even be seen, both worth recording because each produced a confident green: * The liveness probe was `SELECT 1`, which DB2 rejects (SQL0104N — it wants a FROM clause). DB2 was therefore marked unreachable and every DB2 case returned early, reported as **passing** in 0ms while touching nothing. Targets now carry their own probe, and an unreachable engine calls `ctx.skip()` so it can never again be mistaken for a pass. * The post-migration check was a `SELECT`. Reads are exactly what reorg-pending still allows, so it went green against a table the user could no longer write to. It is an INSERT/DELETE now. Verified both directions against DB2 11.5: without the REORG the live suite fails with SQL0668N on the write-back; with it, all 49 cases pass across Postgres, MySQL, MariaDB, SQL Server, CockroachDB, YugabyteDB and DB2. Co-Authored-By: Claude Opus 5 --- .../modules/generated-ddl-live.test.ts | 67 ++++++++++++++++--- .../sql/src/modules/sql-dialect.interface.ts | 18 +++++ .../src/modules/sql-generator.module.test.ts | 51 ++++++++++++++ .../sql/src/modules/sql-generator.module.ts | 18 +++++ .../sql/src/providers/db2/db2.sql-dialect.ts | 17 +++++ 5 files changed, 162 insertions(+), 9 deletions(-) diff --git a/apps/web/src/backend/modules/generated-ddl-live.test.ts b/apps/web/src/backend/modules/generated-ddl-live.test.ts index 888a2ba9..4e991a05 100644 --- a/apps/web/src/backend/modules/generated-ddl-live.test.ts +++ b/apps/web/src/backend/modules/generated-ddl-live.test.ts @@ -31,7 +31,20 @@ const gen = new SqlGeneratorModule(); /** Unique per run, so a rerun never collides with rows an earlier one left. */ const TAG = Date.now().toString(36).slice(-5); -const TARGETS: Array<{ dialect: string; provider: string; options: ConnectionOptions }> = [ +/** + * `probe` is the liveness query, and it is not universal: DB2 rejects a bare + * `SELECT 1` (SQL0104N — it wants a FROM clause), Oracle wants `FROM DUAL`. + * Getting this wrong is worse than it sounds. The first version probed every + * engine with `SELECT 1`, so DB2 was marked unreachable and every DB2 case + * returned early — reported as **passing**, in 0ms, while touching nothing. + * A skip that looks like a pass is the most expensive kind of green there is. + */ +const TARGETS: Array<{ + dialect: string; + provider: string; + options: ConnectionOptions; + probe?: string; +}> = [ { dialect: 'postgres', provider: 'postgres', @@ -69,6 +82,7 @@ const TARGETS: Array<{ dialect: string; provider: string; options: ConnectionOpt dialect: 'db2', provider: 'db2', options: { host: 'localhost', port: 50000, database: 'foxdb', username: 'db2inst1', password: 'foxpass', schema: 'DB2INST1' }, + probe: 'SELECT 1 FROM SYSIBM.SYSDUMMY1', }, ]; @@ -238,7 +252,11 @@ describe.runIf(RUN)('generated DDL runs on the real engines', () => { describe(target.dialect, () => { it('is reachable', async () => { try { - await ConnectionFactory.executeQuery(target.provider, target.options, 'SELECT 1'); + await ConnectionFactory.executeQuery( + target.provider, + target.options, + target.probe ?? 'SELECT 1' + ); reachable.set(target.dialect, true); } catch (err) { reachable.set(target.dialect, false); @@ -247,8 +265,10 @@ describe.runIf(RUN)('generated DDL runs on the real engines', () => { } }); - it('alters a table whose names need quoting', async () => { - if (reachable.get(target.dialect) === false) return; + it('alters a table whose names need quoting', async (ctx) => { + // ctx.skip() rather than `return`: a skipped engine must not report a + // green tick it never earned. + if (reachable.get(target.dialect) === false) ctx.skip(); // ADD / DROP / MODIFY COLUMN go through per-dialect hooks rather than // the CREATE path, so they need their own proof on a real server — // this is where the dialects diverge most. @@ -269,9 +289,11 @@ describe.runIf(RUN)('generated DDL runs on the real engines', () => { primaryKey: { columns: ['id'] }, }); - await runPlan(target, await ddlFor([before], target.dialect), (name) => - toDrop.push({ provider: target.provider, options: target.options, name }) - ); + const created: string[] = []; + await runPlan(target, await ddlFor([before], target.dialect), (name) => { + created.push(name); + toDrop.push({ provider: target.provider, options: target.options, name }); + }); const alters = (await alterDdl([before], [after], target.dialect)).filter( (s) => !s.trim().startsWith('--') @@ -279,11 +301,38 @@ describe.runIf(RUN)('generated DDL runs on the real engines', () => { expect(alters.length, 'compare reported a change but generated no ALTER').toBeGreaterThan(0); // One session for the whole plan, exactly as MigrationModule runs it. await runPlan(target, alters); + + // "The statements ran" is not the same as "the table still works". + // + // It has to be a *write*. DB2 leaves a table in reorg-pending after + // DROP COLUMN, where SELECT still succeeds and every INSERT/UPDATE/ + // DELETE fails with SQL0668N reason code 7 — so a read-back probe went + // green against a table the user could no longer write to. That is the + // failure this case exists to catch, and reading was blind to it. + const quoted = created[created.length - 1]; + expect(quoted, 'no CREATE TABLE captured to write back').toBeTruthy(); + try { + await ConnectionFactory.executeQuery( + target.provider, + target.options, + `INSERT INTO ${quoted} (id) VALUES (4242)` + ); + await ConnectionFactory.executeQuery( + target.provider, + target.options, + `DELETE FROM ${quoted} WHERE id = 4242` + ); + } catch (err) { + throw new Error( + `${target.dialect}: the table cannot be written to after the migration — ` + + `${(err as Error).message.split('\n')[0]}` + ); + } }); for (const testCase of CASES) { - it(`creates ${testCase.label}`, async () => { - if (reachable.get(target.dialect) === false) return; + it(`creates ${testCase.label}`, async (ctx) => { + if (reachable.get(target.dialect) === false) ctx.skip(); const statements = (await ddlFor(testCase.tables, target.dialect)).filter( (s) => !s.trim().startsWith('--') ); diff --git a/packages/sql/src/modules/sql-dialect.interface.ts b/packages/sql/src/modules/sql-dialect.interface.ts index 9c382c58..342473a5 100644 --- a/packages/sql/src/modules/sql-dialect.interface.ts +++ b/packages/sql/src/modules/sql-dialect.interface.ts @@ -131,6 +131,24 @@ export interface SqlDialect { */ quoteIdentifier?(name: string): string; + /** + * Statements to run after a table's columns change, before its indexes, + * keys and triggers are rebuilt. + * + * DB2 puts a table into *reorg-pending* after DROP COLUMN and after some type + * changes: `SELECT` still works, but every write fails with SQL0668N reason + * code 7 until `REORG TABLE` runs. A migration that skips it reports success + * and hands back a table nobody can write to — and because reads still work, + * it can be a while before anyone notices. + * + * Omit unless the engine needs it. Runs inside the migration's transaction + * alongside the ALTERs. + */ + postColumnChangeStatements?( + qualifiedTable: string, + changed: { dropped: boolean; retyped: boolean } + ): string[]; + /** * Full ` COLLATE ...` clause (with leading space) for a column's collation, used in * CREATE TABLE / ADD COLUMN. Default: ` COLLATE `, unquoted — correct for diff --git a/packages/sql/src/modules/sql-generator.module.test.ts b/packages/sql/src/modules/sql-generator.module.test.ts index d3be2dc0..71295557 100644 --- a/packages/sql/src/modules/sql-generator.module.test.ts +++ b/packages/sql/src/modules/sql-generator.module.test.ts @@ -94,6 +94,57 @@ describe('SqlGeneratorModule.generateMigrationPlan', () => { sourceTable: tableSchema({ name, columns: [{ name: 'ID', type: 'INTEGER', nullable: false, primaryKey: false }] }), }); + describe('DB2 reorg-pending after column changes', () => { + // Verified against DB2 11.5: after ALTER TABLE … DROP COLUMN, a SELECT + // still succeeds while every INSERT fails with SQL0668N reason code 7. A + // migration without the REORG therefore reports success and hands back a + // table nobody can write to — and because reads work, it can be a long + // while before anyone notices. + const droppedColumn: TableDiff = { + tableName: 'CUSTOMER', + objectType: 'TABLE', + status: 'MODIFIED', + columnDiffs: [ + { name: 'OLD_COL', status: 'REMOVED', target: { name: 'old_col', type: 'VARCHAR(10)', nullable: true } }, + ], + indexDiffs: [], + foreignKeyDiffs: [], + sourceTable: tableSchema({ name: 'customer' }), + targetTable: tableSchema({ name: 'customer' }), + }; + + it('reorgs the table after a DROP COLUMN', () => { + const sql = gen.generateMigrationPlan([droppedColumn], 'db2').flatMap((s) => s.statements); + const drop = sql.findIndex((s) => /DROP COLUMN/i.test(s)); + const reorg = sql.findIndex((s) => /REORG TABLE/i.test(s)); + expect(drop, 'no DROP COLUMN emitted').toBeGreaterThanOrEqual(0); + expect(reorg, 'no REORG emitted — the table stays unwritable').toBeGreaterThan(drop); + // ADMIN_CMD is the callable form; bare `REORG TABLE` is a CLP command + // and cannot be sent over a client connection. + expect(sql[reorg]).toContain('SYSPROC.ADMIN_CMD'); + }); + + it('does not reorg when nothing was dropped or retyped', () => { + const added: TableDiff = { + ...droppedColumn, + columnDiffs: [ + { name: 'NEW_COL', status: 'ADDED', source: { name: 'new_col', type: 'VARCHAR(10)', nullable: true } }, + ], + }; + const sql = gen.generateMigrationPlan([added], 'db2').flatMap((s) => s.statements).join('\n'); + expect(sql).toMatch(/ADD/i); + expect(sql).not.toMatch(/REORG/i); + }); + + it('leaves other dialects alone', () => { + // Only DB2 implements the hook; nobody else should grow a REORG. + for (const dialect of ['postgres', 'mysql', 'sqlserver', 'oracle', 'sqlite']) { + const sql = gen.generateMigrationPlan([droppedColumn], dialect).flatMap((s) => s.statements).join('\n'); + expect(sql, dialect).not.toMatch(/REORG/i); + } + }); + }); + it('orders steps drop → create → alter', () => { const diffs: TableDiff[] = [ { ...addedTable('NEW') }, diff --git a/packages/sql/src/modules/sql-generator.module.ts b/packages/sql/src/modules/sql-generator.module.ts index ca214d89..d0e3f007 100644 --- a/packages/sql/src/modules/sql-generator.module.ts +++ b/packages/sql/src/modules/sql-generator.module.ts @@ -760,8 +760,12 @@ export class SqlGeneratorModule { statements.push(dialect.addColumnStatement(tableName, colDef)); } + // A type change can also leave a table needing maintenance (see + // `postColumnChangeStatements`), so it is tracked alongside the drops. + let retypedColumns = false; for (const col of obj.columnDiffs.filter((c) => c.status === 'MODIFIED')) { if (!col.source) continue; + if (col.target && col.source.type !== col.target.type) retypedColumns = true; const translated = this.translateType(col.source.type, mapping); if (translated.warning) statements.push(`-- review: ${col.name}: ${translated.warning}`); // Cross-dialect: the raw default AND collation are source-dialect vocabulary @@ -795,12 +799,26 @@ export class SqlGeneratorModule { } } + const droppedColumns = + !mapping?.nonDestructive && obj.columnDiffs.some((c) => c.status === 'REMOVED'); if (!mapping?.nonDestructive) { for (const col of obj.columnDiffs.filter((c) => c.status === 'REMOVED')) { statements.push(dialect.dropColumnStatement(tableName, this.columnIdent(col))); } } + // Before the keys and indexes are rebuilt, not after: an engine that + // needs maintenance here (DB2's REORG) rejects those rebuilds too while + // the table is pending. + if (droppedColumns || retypedColumns) { + statements.push( + ...(dialect.postColumnChangeStatements?.(tableName, { + dropped: droppedColumns, + retyped: retypedColumns, + }) ?? []) + ); + } + const srcPk = obj.sourceTable ? this.primaryKeyColumns(obj.sourceTable) : []; const tgtPk = obj.targetTable ? this.primaryKeyColumns(obj.targetTable) : []; if (!this.sameColumnSet(srcPk, tgtPk)) { diff --git a/packages/sql/src/providers/db2/db2.sql-dialect.ts b/packages/sql/src/providers/db2/db2.sql-dialect.ts index 0ec70d39..d5c163a9 100644 --- a/packages/sql/src/providers/db2/db2.sql-dialect.ts +++ b/packages/sql/src/providers/db2/db2.sql-dialect.ts @@ -93,6 +93,23 @@ export const db2SqlDialect: SqlDialect = { return `ALTER TABLE ${tableName} DROP COLUMN ${colName};`; }, + /** + * DROP COLUMN (and some type changes) leave the table in *reorg-pending*. + * `SELECT` still works, so nothing looks wrong — but every INSERT/UPDATE/ + * DELETE fails with SQL0668N reason code 7, and so does rebuilding the + * table's indexes and keys, until REORG runs. + * + * Verified against DB2 11.5: after `ALTER TABLE … DROP COLUMN`, a SELECT + * succeeded and an INSERT returned SQL0668N. Without this the migration + * reports success and hands back a table nobody can write to. + * + * ADMIN_CMD is the callable form — plain `REORG TABLE` is a CLP command, not + * SQL, and cannot be sent over a client connection. + */ + postColumnChangeStatements(qualifiedTable: string): string[] { + return [`CALL SYSPROC.ADMIN_CMD('REORG TABLE ${qualifiedTable.replace(/'/g, "''")}');`]; + }, + setDefaultStatements(tableName: string, colName: string, defaultValue: string | undefined): string[] { return defaultValue ? [`ALTER TABLE ${tableName} ALTER COLUMN ${colName} SET DEFAULT ${defaultValue};`] From 462f871cfc2673e69d2607c445e5dabddf503da8 Mon Sep 17 00:00:00 2001 From: huyplb Date: Sun, 16 Aug 2026 22:17:45 -0600 Subject: [PATCH 4/7] =?UTF-8?q?fix(db2):=20converge=20on=20the=20real=20sa?= =?UTF-8?q?mples=20=E2=80=94=20phantom=20roles,=20dead=20Sessions,=20drift?= =?UTF-8?q?ing=20defaults?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ran the shipped DEMO_A/DEMO_B samples through the actual app path against DB2 11.5 — provider introspection, CompareModule, generated plan applied to a throwaway schema, then re-introspected and re-compared. Three bugs, each of which a string test could not have seen. **Every DB2 comparison carried six phantom ROLE objects.** `SYSCAT.ROLES` was read unfiltered, so DB2's own built-in roles (SYSDEBUG, SYSGEOADM, SYSTS_*) were reported as user objects in every schema. They cannot be recreated — DB2 reserves the SYS prefix and answers `CREATE ROLE SYSDEBUG` with SQL0707N — so the migration also contained eleven statements that can never succeed. In this database *every* role is a system role, so the whole ROLE section was noise. **Sessions never worked on DB2 at all.** The query selected `SESSION_DB_PARTITION_NUM`, which `MON_GET_CONNECTION` does not expose: DB2 answered SQL0206N and the utility failed outright. It is `CURRENT SERVER` now, which is what the `database_name` column claims to be — a partition number was the wrong value for that slot regardless. **Migrations to DB2 never converged.** DB2 rejects adding a NOT NULL column to a populated table without a default (SQL0193N), so the dialect appends `WITH DEFAULT` — correct, and documented. But that leaves the column holding a default (`''`, `0`) the source never declared, so re-comparing straight after a *successful* migration still reported the column as changed and proposed the same work again, for ever. A new `afterAddColumnStatements` hook drops the implicit default once the rows are backfilled; verified on the server that DROP DEFAULT is accepted immediately after the ADD, needs no REORG between, and leaves the catalog default NULL — matching the source exactly. End state on the samples: 30/30 statements execute, and re-comparing after the migration reports **no differences at all**. All five utilities (pool, sessions, system, sizes, index-fragmentation) run against the live server, and DEMO_A is byte-for-byte untouched by the run. Co-Authored-By: Claude Opus 5 --- packages/db/src/providers/db2/db2.provider.ts | 12 +++- .../src/modules/dialect-dba-utilities.test.ts | 14 +++++ .../sql/src/modules/dialect-dba-utilities.ts | 6 +- .../sql/src/modules/sql-dialect.interface.ts | 14 +++++ .../src/modules/sql-generator.module.test.ts | 57 +++++++++++++++++++ .../sql/src/modules/sql-generator.module.ts | 5 ++ .../sql/src/providers/db2/db2.sql-dialect.ts | 16 ++++++ 7 files changed, 121 insertions(+), 3 deletions(-) diff --git a/packages/db/src/providers/db2/db2.provider.ts b/packages/db/src/providers/db2/db2.provider.ts index c17ea0a4..225da288 100644 --- a/packages/db/src/providers/db2/db2.provider.ts +++ b/packages/db/src/providers/db2/db2.provider.ts @@ -414,14 +414,22 @@ export class Db2Provider implements SchemaProvider { [schemaName] ), // Roles are database-global (not schema-scoped). + // + // `SYS%` is excluded because those are DB2's own built-in roles + // (SYSDEBUG, SYSTS_ADM, SYSGEOADM, …). They exist in every database, + // belong to nobody, and cannot be recreated — DB2 reserves the SYS + // prefix and answers `CREATE ROLE SYSDEBUG` with SQL0707N. Reporting + // them put six phantom objects in every DB2 comparison and put eleven + // statements that can never succeed into the migration. ConnectionFactory.executeOnConnection( this.provider, conn, - `SELECT ROLENAME FROM SYSCAT.ROLES ORDER BY ROLENAME`, + `SELECT ROLENAME FROM SYSCAT.ROLES WHERE ROLENAME NOT LIKE 'SYS%' ORDER BY ROLENAME`, [] ), ConnectionFactory.executeOnConnection( this.provider, conn, - `SELECT ROLENAME, GRANTEE, GRANTEETYPE FROM SYSCAT.ROLEAUTH ORDER BY ROLENAME, GRANTEE`, + `SELECT ROLENAME, GRANTEE, GRANTEETYPE FROM SYSCAT.ROLEAUTH + WHERE ROLENAME NOT LIKE 'SYS%' ORDER BY ROLENAME, GRANTEE`, [] ) ]); diff --git a/packages/sql/src/modules/dialect-dba-utilities.test.ts b/packages/sql/src/modules/dialect-dba-utilities.test.ts index 86fe62e5..5eb4c214 100644 --- a/packages/sql/src/modules/dialect-dba-utilities.test.ts +++ b/packages/sql/src/modules/dialect-dba-utilities.test.ts @@ -112,3 +112,17 @@ describe('dialect-dba-utilities', () => { expect(q.params).toEqual(['CARTER', 'CARTER']); }); }); + +describe('DB2 session query columns', () => { + it('does not select SESSION_DB_PARTITION_NUM', () => { + // MON_GET_CONNECTION has no such column. DB2 answers SQL0206N, so the + // Sessions utility failed for every DB2 user until this was corrected. + const built = buildDbaUtilityQuery({ dialect: 'db2', kind: 'sessions' }); + expect('error' in built).toBe(false); + const sql = (built as { sql: string }).sql; + expect(sql).not.toContain('SESSION_DB_PARTITION_NUM'); + // Verified live on DB2 11.5: this returns the database, which is what the + // column claims to be — a partition number was wrong for it regardless. + expect(sql).toContain('CURRENT SERVER AS database_name'); + }); +}); diff --git a/packages/sql/src/modules/dialect-dba-utilities.ts b/packages/sql/src/modules/dialect-dba-utilities.ts index f72a516c..45dbc63e 100644 --- a/packages/sql/src/modules/dialect-dba-utilities.ts +++ b/packages/sql/src/modules/dialect-dba-utilities.ts @@ -448,6 +448,10 @@ SELECT * FROM ( }; } if (f === 'db2') { + // `database_name` is CURRENT SERVER, not a partition column. The original + // query selected a partition number here under a column name that promised + // the database — and picked one MON_GET_CONNECTION does not expose, so DB2 + // answered SQL0206N and Sessions failed outright for every DB2 user. return { mode: asMode(mode), params: [], @@ -456,7 +460,7 @@ SELECT CAST(APPLICATION_HANDLE AS VARCHAR(32)) AS session_id, SESSION_AUTH_ID AS user_name, CLIENT_HOSTNAME AS client_host, - SESSION_DB_PARTITION_NUM AS database_name, + CURRENT SERVER AS database_name, APPLICATION_NAME AS state, CAST(NULL AS VARCHAR(128)) AS wait_event, CAST(NULL AS VARCHAR(500)) AS query_text, diff --git a/packages/sql/src/modules/sql-dialect.interface.ts b/packages/sql/src/modules/sql-dialect.interface.ts index 342473a5..8a865ff3 100644 --- a/packages/sql/src/modules/sql-dialect.interface.ts +++ b/packages/sql/src/modules/sql-dialect.interface.ts @@ -144,6 +144,20 @@ export interface SqlDialect { * Omit unless the engine needs it. Runs inside the migration's transaction * alongside the ALTERs. */ + /** + * Statements that clean up after this dialect's own `addColumnStatement`. + * + * DB2 cannot add a NOT NULL column to a populated table without a default + * (SQL0193N), so the dialect appends `WITH DEFAULT` to make the ALTER + * succeed. That leaves the column carrying a default (`''`, `0`) the source + * column never had — so the very next comparison reports the column as + * changed, proposes the same migration again, and never converges. Dropping + * the implicit default afterwards makes the result actually match the source. + * + * Omit unless `addColumnStatement` adds something the source did not ask for. + */ + afterAddColumnStatements?(qualifiedTable: string, colName: string, col: ColumnSpec): string[]; + postColumnChangeStatements?( qualifiedTable: string, changed: { dropped: boolean; retyped: boolean } diff --git a/packages/sql/src/modules/sql-generator.module.test.ts b/packages/sql/src/modules/sql-generator.module.test.ts index 71295557..f0144db4 100644 --- a/packages/sql/src/modules/sql-generator.module.test.ts +++ b/packages/sql/src/modules/sql-generator.module.test.ts @@ -94,6 +94,63 @@ describe('SqlGeneratorModule.generateMigrationPlan', () => { sourceTable: tableSchema({ name, columns: [{ name: 'ID', type: 'INTEGER', nullable: false, primaryKey: false }] }), }); + describe('DB2 converges after adding a NOT NULL column', () => { + // DB2 cannot add a NOT NULL column to a populated table without a default + // (SQL0193N), so the dialect appends `WITH DEFAULT`. That leaves the column + // holding a default the source never declared, and the next comparison + // reports the same change again — for ever. Verified end to end against the + // shipped DEMO_A/DEMO_B samples on DB2 11.5: with this, re-comparing after + // the migration reports no differences at all. + const addNotNull = (over: Partial<{ nullable: boolean; defaultValue: string }> = {}): TableDiff => ({ + tableName: 'PRODUCTS', + objectType: 'TABLE', + status: 'MODIFIED', + columnDiffs: [ + { + name: 'SKU', + status: 'ADDED', + source: { name: 'SKU', type: 'VARCHAR(50)', nullable: false, ...over }, + }, + ], + indexDiffs: [], + foreignKeyDiffs: [], + sourceTable: tableSchema({ name: 'PRODUCTS' }), + targetTable: tableSchema({ name: 'PRODUCTS' }), + }); + + it('drops the implicit default it had to add', () => { + const sql = gen.generateMigrationPlan([addNotNull()], 'db2').flatMap((s) => s.statements); + const add = sql.findIndex((s) => /ADD\b/i.test(s) && /WITH DEFAULT/i.test(s)); + const drop = sql.findIndex((s) => /DROP DEFAULT/i.test(s)); + expect(add, 'no ADD … WITH DEFAULT emitted').toBeGreaterThanOrEqual(0); + expect(drop, 'the implicit default is never dropped — this never converges').toBeGreaterThan(add); + }); + + it('keeps a default the source actually declares', () => { + const sql = gen + .generateMigrationPlan([addNotNull({ defaultValue: "'x'" })], 'db2') + .flatMap((s) => s.statements) + .join('\n'); + expect(sql).toContain("DEFAULT 'x'"); + expect(sql, 'dropped a default the source asked for').not.toMatch(/DROP DEFAULT/i); + }); + + it('leaves a nullable column alone', () => { + const sql = gen + .generateMigrationPlan([addNotNull({ nullable: true })], 'db2') + .flatMap((s) => s.statements) + .join('\n'); + expect(sql).not.toMatch(/DROP DEFAULT/i); + }); + + it('does not touch other dialects', () => { + for (const dialect of ['postgres', 'mysql', 'sqlserver', 'oracle']) { + const sql = gen.generateMigrationPlan([addNotNull()], dialect).flatMap((s) => s.statements).join('\n'); + expect(sql, dialect).not.toMatch(/DROP DEFAULT/i); + } + }); + }); + describe('DB2 reorg-pending after column changes', () => { // Verified against DB2 11.5: after ALTER TABLE … DROP COLUMN, a SELECT // still succeeds while every INSERT fails with SQL0668N reason code 7. A diff --git a/packages/sql/src/modules/sql-generator.module.ts b/packages/sql/src/modules/sql-generator.module.ts index d0e3f007..2bd017fe 100644 --- a/packages/sql/src/modules/sql-generator.module.ts +++ b/packages/sql/src/modules/sql-generator.module.ts @@ -758,6 +758,11 @@ export class SqlGeneratorModule { if (!col.source.nullable) colDef += ` NOT NULL`; } statements.push(dialect.addColumnStatement(tableName, colDef)); + // Let the dialect clean up after its own workaround (DB2's implicit + // `WITH DEFAULT`), or the migration never converges. + statements.push( + ...(dialect.afterAddColumnStatements?.(tableName, this.columnIdent(col), col.source) ?? []) + ); } // A type change can also leave a table needing maintenance (see diff --git a/packages/sql/src/providers/db2/db2.sql-dialect.ts b/packages/sql/src/providers/db2/db2.sql-dialect.ts index d5c163a9..84b089a8 100644 --- a/packages/sql/src/providers/db2/db2.sql-dialect.ts +++ b/packages/sql/src/providers/db2/db2.sql-dialect.ts @@ -80,6 +80,22 @@ export const db2SqlDialect: SqlDialect = { return `ALTER TABLE ${tableName} ADD ${def};`; }, + /** + * Undo the `WITH DEFAULT` above once the rows are backfilled. + * + * Without this the new column keeps a default (`''`, `0`) the source column + * never declared, so re-comparing straight after a *successful* migration + * still reports the column as changed — the tool proposes the same migration + * for ever and never converges. Verified on DB2 11.5: DROP DEFAULT is + * accepted immediately after the ADD, needs no REORG in between, and leaves + * the catalog default NULL, matching the source exactly. + */ + afterAddColumnStatements(tableName: string, colName: string, col: ColumnSpec): string[] { + const sourceHadDefault = col.defaultValue !== undefined && col.defaultValue !== null; + if (col.nullable || sourceHadDefault) return []; + return [`ALTER TABLE ${tableName} ALTER COLUMN ${colName} DROP DEFAULT;`]; + }, + modifyColumnStatements(tableName: string, colName: string, col: ColumnSpec): string[] { const stmts = [`ALTER TABLE ${tableName} ALTER COLUMN ${colName} SET DATA TYPE ${col.type};`]; // DB2 nullability is a separate clause — SET DATA TYPE does not carry it. From 5f69cc7d0fd48b5aebf800f0e79da89c7ccb0751 Mon Sep 17 00:00:00 2001 From: huyplb Date: Sun, 16 Aug 2026 22:25:12 -0600 Subject: [PATCH 5/7] fix(db2): a revert that hits an already-dropped FK no longer rolls itself back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drove the whole product loop against real DB2 for the first time: seed a scratch schema with MigrationModule, capture v1 with Lokee, migrate DEMO_A into it, capture v2, diff the versions, plan a revert, execute it, and check the database actually went back. It did not. The revert reported success and changed nothing. Table drops are ordered before the ALTERs, and dropping a parent table takes its inbound foreign keys with it — so `ALTER TABLE … DROP FOREIGN KEY` ran against a constraint DB2 had already removed and raised SQL0204N. DB2 has transactional DDL, so that one statement rolled the *entire* revert back. Everything else in the plan was correct; none of it survived. The dialect already knows this shape: its DROP TABLE/VIEW go through a SQL PL `CONTINUE HANDLER FOR SQLSTATE '42704'` because DB2 has no DROP IF EXISTS. The FK drop simply never got the same treatment, even though the generic fallback it overrides says `DROP CONSTRAINT IF EXISTS` for exactly this reason. It is wrapped now. Verified end to end on DB2 11.5: after the revert the live schema matches v1 exactly, and the new version's root hash equals v1's — content-addressed proof that what came back is identical, not merely similar. One note for whoever writes the next harness: `MigrationModule.execute` reports failure through its **event stream** (`{type:'done', success:false, rolledBack:true}`), not by throwing. My first pass wrapped it in try/catch, saw no exception, and cheerfully reported a rolled-back revert as applied. Co-Authored-By: Claude Opus 5 --- .../src/modules/sql-generator.module.test.ts | 20 +++++++++++++++++++ .../sql/src/providers/db2/db2.sql-dialect.ts | 16 ++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/sql/src/modules/sql-generator.module.test.ts b/packages/sql/src/modules/sql-generator.module.test.ts index f0144db4..a2f6e6c3 100644 --- a/packages/sql/src/modules/sql-generator.module.test.ts +++ b/packages/sql/src/modules/sql-generator.module.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from 'vitest'; import { SqlGeneratorModule } from './sql-generator.module.js'; +import { db2SqlDialect } from '../providers/db2/db2.sql-dialect.js'; import { TableDiff } from '../interfaces/index.js'; import { TableSchema } from '../interfaces/index.js'; @@ -94,6 +95,25 @@ describe('SqlGeneratorModule.generateMigrationPlan', () => { sourceTable: tableSchema({ name, columns: [{ name: 'ID', type: 'INTEGER', nullable: false, primaryKey: false }] }), }); + describe('DB2 tolerates a foreign key that is already gone', () => { + it('wraps the FK drop in the same 42704 handler as its other drops', () => { + // Dropping a parent table takes its inbound foreign keys with it, and + // table drops are ordered before the ALTERs — so by the time the explicit + // FK drop runs, the constraint often no longer exists. DB2 has no DROP + // CONSTRAINT IF EXISTS, and DB2 has transactional DDL, so the resulting + // SQL0204N rolled the *entire* migration back: on the live samples the + // revert reported no error and changed nothing at all. + const sql = db2SqlDialect.dropForeignKeyStatement!('S.CHILD', 'FK_CHILD_PARENT'); + expect(sql).toContain("SQLSTATE '42704'"); + expect(sql).toContain('DROP FOREIGN KEY FK_CHILD_PARENT'); + }); + + it('still drops the constraint it was asked to drop', () => { + const sql = db2SqlDialect.dropForeignKeyStatement!('S.CHILD', 'FK_X'); + expect(sql).toMatch(/ALTER TABLE S\.CHILD DROP FOREIGN KEY FK_X/); + }); + }); + describe('DB2 converges after adding a NOT NULL column', () => { // DB2 cannot add a NOT NULL column to a populated table without a default // (SQL0193N), so the dialect appends `WITH DEFAULT`. That leaves the column diff --git a/packages/sql/src/providers/db2/db2.sql-dialect.ts b/packages/sql/src/providers/db2/db2.sql-dialect.ts index 84b089a8..2fc262e1 100644 --- a/packages/sql/src/providers/db2/db2.sql-dialect.ts +++ b/packages/sql/src/providers/db2/db2.sql-dialect.ts @@ -64,6 +64,12 @@ function db2Drop(keyword: string, name: string): string { return `BEGIN\n DECLARE CONTINUE HANDLER FOR SQLSTATE '42704' BEGIN END;\n EXECUTE IMMEDIATE 'DROP ${keyword} ${safe}';\nEND`; } +/** Same tolerance for a constraint, which lives on ALTER TABLE rather than DROP. */ +function db2DropConstraint(tableName: string, fkName: string): string { + const safe = `ALTER TABLE ${tableName} DROP FOREIGN KEY ${fkName}`.replace(/'/g, "''"); + return `BEGIN\n DECLARE CONTINUE HANDLER FOR SQLSTATE '42704' BEGIN END;\n EXECUTE IMMEDIATE '${safe}';\nEND`; +} + export const db2SqlDialect: SqlDialect = { identityClause(c: ColumnSpec): string { return c.identity ? ` GENERATED ${c.identityGeneration ?? 'ALWAYS'} AS IDENTITY` : ''; @@ -138,7 +144,15 @@ export const db2SqlDialect: SqlDialect = { dropForeignKeyStatement(tableName: string, fkName: string): string { // DB2 has no DROP CONSTRAINT IF EXISTS; DROP FOREIGN KEY is the native form. - return `ALTER TABLE ${tableName} DROP FOREIGN KEY ${fkName};`; + // + // Wrapped in the same 42704 handler as the other drops, because the + // constraint may already be gone by the time this runs: dropping a parent + // table earlier in the plan takes its inbound foreign keys with it. Without + // the handler that raised SQL0204N, and since DB2 has transactional DDL the + // *whole* migration rolled back — a revert that reported no error and + // changed nothing. The generic fallback says `DROP CONSTRAINT IF EXISTS` + // for exactly this reason; this restores that tolerance for DB2. + return `${db2DropConstraint(tableName, fkName)};`; }, dropIndexStatement(indexName: string, qualifiedTable: string): string { From 514c65c963607b41934a9160d9463419875ed2e4 Mon Sep 17 00:00:00 2001 From: huyplb Date: Sun, 16 Aug 2026 22:36:28 -0600 Subject: [PATCH 6/7] test(sql): round-trip procedures and functions on the live engines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Routine bodies are the least portable thing in SQL and nothing here exercised them: the DB2 samples contain no routines at all (its seed's compound blocks are only drop-cleanup), so DEMO_A/DEMO_B never tested a single one. Adds the round trip, which is the only version of this test worth running: create a function and a procedure with native DDL, read them back through the provider, ask the generator to recreate them in a *second* schema, and execute that. Asserting "the object exists" would pass on a definition that is empty, missing its terminator, or has the source schema baked into it; executing the regenerated DDL somewhere else catches all three. Covers Postgres, MySQL, SQL Server and DB2 — each with its own body, since there is no portable one. Engines without a spec skip visibly rather than reporting a green they did not earn. All four pass: the captured definition is faithful enough to rebuild the routine elsewhere. MySQL needs the second *database* (a MySQL schema is a database), which the demo user has no rights to create, so that one target carries admin credentials — an environment limit, not a product one. Co-Authored-By: Claude Opus 5 --- .../modules/generated-ddl-live.test.ts | 136 +++++++++++++++++- 1 file changed, 135 insertions(+), 1 deletion(-) diff --git a/apps/web/src/backend/modules/generated-ddl-live.test.ts b/apps/web/src/backend/modules/generated-ddl-live.test.ts index 4e991a05..11dfaa4e 100644 --- a/apps/web/src/backend/modules/generated-ddl-live.test.ts +++ b/apps/web/src/backend/modules/generated-ddl-live.test.ts @@ -21,7 +21,7 @@ * compose file but slow to boot; add them here once they are healthy. */ import { afterAll, describe, expect, it } from 'vitest'; -import { ConnectionFactory, getAdapter } from '@foxschema/db'; +import { ConnectionFactory, getAdapter, getRegisteredProvider } from '@foxschema/db'; import { CompareModule, SqlGeneratorModule } from '@foxschema/sql'; import type { ConnectionOptions, TableSchema } from '@foxschema/sql'; @@ -247,6 +247,65 @@ afterAll(async () => { } }); +/** + * Native DDL for one function and one procedure, plus where to recreate them. + * + * Routine bodies are the least portable thing in SQL, so each engine gets its + * own. Engines absent here have no routine coverage rather than a pretend one. + */ +const ROUTINES: Record< + string, + { + from: string; + to: string; + makeSchema?: (s: string) => string[]; + ddl: (s: string) => string[]; + /** Credentials with rights the demo user lacks (creating a database). */ + admin?: Partial; + } +> = { + postgres: { + from: `fx_a_${TAG}`, + to: `fx_b_${TAG}`, + makeSchema: (s) => [`CREATE SCHEMA IF NOT EXISTS ${s}`], + ddl: (s) => [ + `CREATE FUNCTION ${s}.double_it(x integer) RETURNS integer AS $$ SELECT x * 2 $$ LANGUAGE sql`, + `CREATE PROCEDURE ${s}.touch_it() LANGUAGE plpgsql AS $$ BEGIN PERFORM 1; END; $$`, + ], + }, + mysql: { + // A MySQL schema *is* a database, so the round trip needs a second one. + from: 'foxdb', + to: `foxb_${TAG}`, + // The demo user cannot create a database ("Access denied for user + // 'foxuser'"), and this round trip needs a second one to recreate into. + admin: { username: 'root', password: 'foxrootpass' }, + makeSchema: (s) => [`CREATE DATABASE IF NOT EXISTS ${s}`], + ddl: () => [ + `CREATE FUNCTION double_it_${TAG}(x INT) RETURNS INT DETERMINISTIC RETURN x * 2`, + `CREATE PROCEDURE touch_it_${TAG}() BEGIN SELECT 1; END`, + ], + }, + sqlserver: { + from: 'dbo', + to: `foxb_${TAG}`, + makeSchema: (s) => (s === 'dbo' ? [] : [`IF SCHEMA_ID('${s}') IS NULL EXEC('CREATE SCHEMA ${s}')`]), + ddl: () => [ + `CREATE FUNCTION dbo.double_it_${TAG}(@x INT) RETURNS INT AS BEGIN RETURN @x * 2 END`, + `CREATE PROCEDURE dbo.touch_it_${TAG} AS BEGIN SELECT 1 END`, + ], + }, + db2: { + from: `FXA${TAG}`, + to: `FXB${TAG}`, + makeSchema: (s) => [`CREATE SCHEMA ${s}`], + ddl: (s) => [ + `CREATE FUNCTION ${s}.DOUBLE_IT(X INTEGER) RETURNS INTEGER LANGUAGE SQL RETURN X * 2`, + `CREATE PROCEDURE ${s}.TOUCH_IT() LANGUAGE SQL BEGIN DECLARE V INT; SET V = 1; END`, + ], + }, +}; + describe.runIf(RUN)('generated DDL runs on the real engines', () => { for (const target of TARGETS) { describe(target.dialect, () => { @@ -265,6 +324,81 @@ describe.runIf(RUN)('generated DDL runs on the real engines', () => { } }); + const routines = ROUTINES[target.dialect]; + it.runIf(routines)('captures a procedure and a function well enough to recreate them', async (ctx) => { + if (reachable.get(target.dialect) === false) ctx.skip(); + const spec = routines!; + const opts = (schema: string) => ({ + ...target.options, + ...spec.admin, + ...(target.dialect === 'mysql' ? { database: schema } : {}), + schema, + }); + + const admin = await ConnectionFactory.create(target.provider, opts(spec.from), { pooled: false }); + const adapter = getAdapter(target.provider); + const exec = (conn: unknown, sql: string) => adapter.query(conn as never, sql, []); + try { + for (const schema of [spec.from, spec.to]) + for (const stmt of spec.makeSchema?.(schema) ?? []) await exec(admin, stmt).catch(() => undefined); + for (const stmt of spec.ddl(spec.from)) await exec(admin, stmt); + + // Read them back the way the app does. + const provider = getRegisteredProvider(target.provider)!; + const objects = (await provider.getTables!(opts(spec.from), spec.from)) as TableSchema[]; + const mine = objects.filter( + (o) => + (o.objectType === 'FUNCTION' || o.objectType === 'PROCEDURE') && + new RegExp(`(double_it|touch_it)(_${TAG})?$`, 'i').test(o.name) + ); + expect(mine.map((r) => r.objectType).sort()).toEqual(['FUNCTION', 'PROCEDURE']); + + // A routine with no body cannot be migrated anywhere, and an empty + // string would still pass a "the object exists" assertion. + for (const r of mine) { + expect((r.definition ?? '').trim().length, `${r.name} was captured without a body`).toBeGreaterThan(0); + } + + // The round trip: regenerate into the other schema and run it. This + // is what catches a definition that is missing its terminator or has + // the source schema baked into it. + const compare = await new CompareModule().compare(mine, [], { + source: target.dialect, + target: target.dialect, + }); + const stmts = gen + .generateMigrationPlan(compare.tables, target.dialect, { sourceSchema: spec.from, targetSchema: spec.to }) + .flatMap((s) => s.statements) + .filter((s) => !s.trim().startsWith('--')); + expect(stmts.length, 'no DDL generated for the routines').toBeGreaterThan(0); + + const other = await ConnectionFactory.create(target.provider, opts(spec.to), { pooled: false }); + try { + for (const raw of stmts) { + const sql = raw.replace(/;\s*$/, ''); + try { + await exec(other, sql); + } catch (err) { + throw new Error( + `${target.dialect} rejected the regenerated routine:\n${sql}\n\n${(err as Error).message.split('\n')[0]}` + ); + } + } + } finally { + await ConnectionFactory.close(target.provider, other).catch(() => undefined); + } + } finally { + // Routines are dropped by name; the schemas themselves are left for + // the engine's own cleanup since each run uses a fresh TAG. + for (const stmt of spec.ddl(spec.from)) { + const kind = /FUNCTION/i.test(stmt) ? 'FUNCTION' : 'PROCEDURE'; + const name = stmt.match(/CREATE (?:FUNCTION|PROCEDURE)\s+(\S+?)\s*[(\s]/i)?.[1]; + if (name) await exec(admin, `DROP ${kind} ${name}`).catch(() => undefined); + } + await ConnectionFactory.close(target.provider, admin).catch(() => undefined); + } + }); + it('alters a table whose names need quoting', async (ctx) => { // ctx.skip() rather than `return`: a skipped engine must not report a // green tick it never earned. From 45b2016e52c446fa4c4a57c54730fc862c7653cc Mon Sep 17 00:00:00 2001 From: huyplb Date: Sun, 16 Aug 2026 22:44:20 -0600 Subject: [PATCH 7/7] chore(sync): drop the bindings left behind when Compare lost its Browse buttons `browseSchema`, `isBrowsing`, `dialectOptions` and the `PROVIDER_SETTINGS` import stopped being referenced when Browse became its own pane and the buttons came out of Compare. Dead either way, but they read as if Compare still has a browse path. Co-Authored-By: Claude Opus 5 --- apps/web/src/frontend/components/TopToolbar.tsx | 5 ----- 1 file changed, 5 deletions(-) diff --git a/apps/web/src/frontend/components/TopToolbar.tsx b/apps/web/src/frontend/components/TopToolbar.tsx index 7900d2b3..8073316e 100644 --- a/apps/web/src/frontend/components/TopToolbar.tsx +++ b/apps/web/src/frontend/components/TopToolbar.tsx @@ -10,7 +10,6 @@ import { CredentialManager } from './CredentialManager'; import { MigrationHistory } from './MigrationHistory'; import { TYPE_META, TYPE_ORDER } from './SchemaTreePanel'; import type { DbObjectType } from '../lib/types'; -import { PROVIDER_SETTINGS } from '../lib/provider-settings'; import { ConnectionModal } from './ConnectionModal'; import { PasswordInput } from './PasswordInput'; import { useAuthStore } from '../store/authStore'; @@ -22,8 +21,6 @@ import { BrowseBar } from './BrowseBar'; const ProfileMenu = ProfileMenuNamed ?? ProfileMenuDefault; -const dialectOptions = Object.values(PROVIDER_SETTINGS); - export const TopToolbar: React.FC = () => { const { sourceConfig, @@ -38,8 +35,6 @@ export const TopToolbar: React.FC = () => { testTargetConnection, isComparing, runSchemaComparison, - browseSchema, - isBrowsing, compareResult, resetSync, selectedObjectTypes,