From 709e03e658f6bd02f3d6a593451d52aeeee157e0 Mon Sep 17 00:00:00 2001 From: tristantr Date: Mon, 13 Jul 2026 14:55:28 +0200 Subject: [PATCH] fix(hub-projects): make generated data table migrations idempotent Co-Authored-By: Claude Fable 5 --- .../src/lib/components/datatableSchemaSql.ts | 16 +++++++-- .../projectMigrations.test.ts | 34 +++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/frontend/src/lib/components/datatableSchemaSql.ts b/frontend/src/lib/components/datatableSchemaSql.ts index b21b51e121..9af9a8edff 100644 --- a/frontend/src/lib/components/datatableSchemaSql.ts +++ b/frontend/src/lib/components/datatableSchemaSql.ts @@ -172,14 +172,26 @@ export function generateMigrationSql( const pkLine = pkCols.length > 0 ? `,\n PRIMARY KEY (${pkCols.join(', ')})` : '' const qualifiedName = `"${change.schemaName}"."${change.tableName}"` const createKeyword = options?.ifNotExists ? 'CREATE TABLE IF NOT EXISTS' : 'CREATE TABLE' - let sql = `BEGIN;\n${createKeyword} ${qualifiedName} (\n ${colDefs}${pkLine}\n);` + // The target may not have the schema at all (fresh data table import). + const schemaDdl = + change.schemaName !== 'public' ? `CREATE SCHEMA IF NOT EXISTS "${change.schemaName}";\n` : '' + let sql = `BEGIN;\n${schemaDdl}${createKeyword} ${qualifiedName} (\n ${colDefs}${pkLine}\n);` for (const fk of table.foreignKeys ?? []) { const fkSql = renderForeignKey(fk, { useSchema: true, dbType: 'postgresql', tableName: change.tableName }) - sql += `\nALTER TABLE ${qualifiedName} ADD ${fkSql};` + // With IF NOT EXISTS the table may pre-exist with this FK already in + // place; an unconditional ADD would then abort the whole transaction. + // The constraint name is emitted unquoted, so Postgres folds it to + // lowercase — compare against the folded form. + const fkName = options?.ifNotExists + ? fkSql.match(/^CONSTRAINT\s+(\S+)/)?.[1]?.toLowerCase() + : undefined + sql += fkName + ? `\nDO $$\nBEGIN\n IF NOT EXISTS (\n SELECT 1 FROM pg_constraint\n WHERE conname = '${fkName}' AND conrelid = '${qualifiedName}'::regclass\n ) THEN\n ALTER TABLE ${qualifiedName} ADD ${fkSql};\n END IF;\nEND $$;` + : `\nALTER TABLE ${qualifiedName} ADD ${fkSql};` } sql += '\nCOMMIT;' return sql diff --git a/frontend/src/lib/components/workspaceSettings/projectMigrations.test.ts b/frontend/src/lib/components/workspaceSettings/projectMigrations.test.ts index ce972f8e31..631cef8c55 100644 --- a/frontend/src/lib/components/workspaceSettings/projectMigrations.test.ts +++ b/frontend/src/lib/components/workspaceSettings/projectMigrations.test.ts @@ -160,6 +160,40 @@ describe('generateDatatableMigrations', () => { expect(migrations[0].sql).toContain('"public"."customers"') }) + it('guards FK creation so re-running on an existing table does not abort', async () => { + getDatatableFullSchemaMock.mockResolvedValue(schema) + const usage = new Map([['main', new Set(['orders'])]]) + const migrations = await generateDatatableMigrations('ws', usage) + const sql = migrations[0].sql + // The ADD CONSTRAINT must be wrapped in a pg_constraint existence check. + expect(sql).toContain('DO $$') + expect(sql).toContain('SELECT 1 FROM pg_constraint') + expect(sql).toContain(`conrelid = '"public"."orders"'::regclass`) + // No unguarded ALTER TABLE ... ADD at the start of a line. + expect(/^ALTER TABLE .* ADD CONSTRAINT/m.test(sql)).toBe(false) + }) + + it('creates non-public schemas before their tables', async () => { + const appSchema = { + app: { + customers: { + name: 'customers', + columns: [{ name: 'id', datatype: 'integer', primary_key: true, nullable: false }], + foreign_keys: [] + } + } + } + getDatatableFullSchemaMock.mockResolvedValue(appSchema) + const usage = new Map([['main', new Set(['app.customers'])]]) + const migrations = await generateDatatableMigrations('ws', usage) + const sql = migrations[0].sql + expect(sql).toContain('CREATE SCHEMA IF NOT EXISTS "app";') + expect(sql.indexOf('CREATE SCHEMA IF NOT EXISTS "app";')).toBeLessThan( + sql.indexOf('CREATE TABLE IF NOT EXISTS "app"."customers"') + ) + expect(sql).not.toContain('CREATE SCHEMA IF NOT EXISTS "public"') + }) + it('keeps same-named tables from different schemas both created', async () => { const twoSchemas = { public: {