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..11dfaa4e --- /dev/null +++ b/apps/web/src/backend/modules/generated-ddl-live.test.ts @@ -0,0 +1,482 @@ +/** + * 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, getAdapter, getRegisteredProvider } 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); + +/** + * `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', + 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' }, + }, + { + // 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' }, + probe: 'SELECT 1 FROM SYSIBM.SYSDUMMY1', + }, +]; + +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); +} + +/** + * 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 }> = []; + +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); + } +}); + +/** + * 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, () => { + it('is reachable', async () => { + try { + await ConnectionFactory.executeQuery( + target.provider, + target.options, + target.probe ?? '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]}`); + } + }); + + 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. + 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. + 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'] }, + }); + + 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('--') + ); + 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 (ctx) => { + if (reachable.get(target.dialect) === false) ctx.skip(); + const statements = (await ddlFor(testCase.tables, target.dialect)).filter( + (s) => !s.trim().startsWith('--') + ); + expect(statements.length, 'generated no DDL to execute').toBeGreaterThan(0); + + await runPlan(target, statements, (name) => + toDrop.push({ provider: target.provider, options: target.options, name }) + ); + }); + } + }); + } +}); 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, 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 9c382c58..8a865ff3 100644 --- a/packages/sql/src/modules/sql-dialect.interface.ts +++ b/packages/sql/src/modules/sql-dialect.interface.ts @@ -131,6 +131,38 @@ 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. + */ + /** + * 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 } + ): 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..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,133 @@ 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 + // 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 + // 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..2bd017fe 100644 --- a/packages/sql/src/modules/sql-generator.module.ts +++ b/packages/sql/src/modules/sql-generator.module.ts @@ -758,10 +758,19 @@ 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 + // `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 +804,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..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` : ''; @@ -80,6 +86,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. @@ -93,6 +115,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};`] @@ -105,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 {