From 69fd71c44fb3fe776909a5243292617aa34a2063 Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Fri, 21 Aug 2026 10:17:36 +0200 Subject: [PATCH] fix: qualify a data table FK target with its schema Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HtYCxXEn2WujwVvh5aZRCa --- .../display/dbtable/queries/dbQueriesUtils.ts | 19 +++++++- .../src/lib/components/datatableSchemaSql.ts | 35 ++++++++++++--- .../projectMigrations.test.ts | 44 ++++++++++++++++++- 3 files changed, 90 insertions(+), 8 deletions(-) diff --git a/frontend/src/lib/components/apps/components/display/dbtable/queries/dbQueriesUtils.ts b/frontend/src/lib/components/apps/components/display/dbtable/queries/dbQueriesUtils.ts index 711fb7ed38..776c48f2b8 100644 --- a/frontend/src/lib/components/apps/components/display/dbtable/queries/dbQueriesUtils.ts +++ b/frontend/src/lib/components/apps/components/display/dbtable/queries/dbQueriesUtils.ts @@ -1,5 +1,6 @@ import type { DbType } from '$lib/components/dbTypes' import type { TableEditorForeignKey, TableEditorValuesColumn } from '../tableEditor' +import { renderDbQuotedIdentifier } from '../utils' export function formatDefaultValue(str: string, datatype: string, resourceType: DbType): string { if (!str) return '' @@ -33,6 +34,12 @@ export function renderForeignKey( useSchema: boolean dbType: DbType tableName: string + /** + * Quotes each dot-separated part of the target in the REFERENCES clause, so a + * schema-qualified target survives identifiers that need quoting. The constraint + * name is built from the unquoted value either way. + */ + quoteTarget?: boolean } ): string { const sourceColumns = fk.columns.map((c) => c.sourceColumn).filter(Boolean) @@ -53,9 +60,17 @@ export function renderForeignKey( .join('_') .replaceAll('.', '_')} `.substring(0, 60) - sql += ` FOREIGN KEY (${sourceColumns.join( + const targetRef = + options.quoteTarget && targetTable + ? targetTable + .split('.') + .map((part) => renderDbQuotedIdentifier(part, options.dbType)) + .join('.') + : targetTable + + sql += ` FOREIGN KEY (${sourceColumns.join(', ')}) REFERENCES ${targetRef} (${targetColumns.join( ', ' - )}) REFERENCES ${targetTable} (${targetColumns.join(', ')})` + )})` if (fk.onDelete !== 'NO ACTION') sql += ` ON DELETE ${fk.onDelete}` if (fk.onUpdate !== 'NO ACTION') sql += ` ON UPDATE ${fk.onUpdate}` return sql diff --git a/frontend/src/lib/components/datatableSchemaSql.ts b/frontend/src/lib/components/datatableSchemaSql.ts index cd3d0f9154..bcf868c04f 100644 --- a/frontend/src/lib/components/datatableSchemaSql.ts +++ b/frontend/src/lib/components/datatableSchemaSql.ts @@ -151,6 +151,27 @@ function resolveColumnType(c: TableEditorValuesColumn): { * callers creating several tables can emit every CREATE before any constraint — * required for circular FKs, where no creation order satisfies inline FKs. */ +/** + * The schema API reports a foreign key's target as a bare table name whenever it + * lives in the same schema as the table declaring it. Emitting that verbatim + * produces `REFERENCES links (id)`, which Postgres resolves against `search_path` + * — so a migration that just created `"bitly"."links"` fails with `relation + * "links" does not exist`. Qualify it: the declaring table's own schema first, + * then any schema that has the table, matching how the FK closure resolves it. + */ +function qualifyFkTarget( + sourceSchema: DatabaseSchema, + targetTable: string | undefined, + declaringSchema: string +): string | undefined { + if (!targetTable || targetTable.includes('.')) return targetTable + if (sourceSchema[declaringSchema]?.[targetTable]) return `${declaringSchema}.${targetTable}` + for (const schemaName of Object.keys(sourceSchema)) { + if (sourceSchema[schemaName]?.[targetTable]) return `${schemaName}.${targetTable}` + } + return targetTable +} + export function generateAddedTableSql( change: TableDiff, sourceSchema: DatabaseSchema, @@ -177,11 +198,15 @@ export function generateAddedTableSql( const create = `${schemaDdl}${createKeyword} ${qualifiedName} (\n ${colDefs}${pkLine}\n);` const constraints: string[] = [] for (const fk of table.foreignKeys ?? []) { - const fkSql = renderForeignKey(fk, { - useSchema: true, - dbType: 'postgresql', - tableName: change.tableName - }) + const fkSql = renderForeignKey( + { ...fk, targetTable: qualifyFkTarget(sourceSchema, fk.targetTable, change.schemaName) }, + { + useSchema: true, + dbType: 'postgresql', + tableName: change.tableName, + quoteTarget: true + } + ) // 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 diff --git a/frontend/src/lib/components/workspaceSettings/projectMigrations.test.ts b/frontend/src/lib/components/workspaceSettings/projectMigrations.test.ts index 7d35c51650..303daf5f8b 100644 --- a/frontend/src/lib/components/workspaceSettings/projectMigrations.test.ts +++ b/frontend/src/lib/components/workspaceSettings/projectMigrations.test.ts @@ -184,7 +184,7 @@ describe('generateDatatableMigrations', () => { expect(migrations[0].sql).toContain('"public"."customers"') }) - it('leaves a qualified ref unresolved when its schema misses, never another schema\'s table', async () => { + it("leaves a qualified ref unresolved when its schema misses, never another schema's table", async () => { getDatatableFullSchemaMock.mockResolvedValue(schema) const usage = new Map([['main', new Set(['sales.orders'])]]) const migrations = await generateDatatableMigrations('ws', usage) @@ -275,6 +275,48 @@ describe('generateDatatableMigrations', () => { expect(sql).not.toContain('CREATE SCHEMA IF NOT EXISTS "public"') }) + it('qualifies an FK target the schema reports bare, so it resolves outside search_path', async () => { + // The schema API omits the schema on an FK target that lives in the same schema + // as the table declaring it. Emitted verbatim it becomes `REFERENCES links (id)`, + // which Postgres looks up on `search_path` — and the migration's own schema is + // never on it, so applying it fails with `relation "links" does not exist`. + const bitlySchema = { + bitly: { + links: { + name: 'links', + columns: [{ name: 'id', datatype: 'uuid', primary_key: true, nullable: false }], + foreign_keys: [] + }, + clicks: { + name: 'clicks', + columns: [ + { name: 'id', datatype: 'uuid', primary_key: true, nullable: false }, + { name: 'link_id', datatype: 'uuid', nullable: false } + ], + foreign_keys: [ + { + target_table: 'links', + columns: [{ source_column: 'link_id', target_column: 'id' }], + on_delete: 'CASCADE', + on_update: 'NO ACTION' + } + ] + } + } + } + getDatatableFullSchemaMock.mockResolvedValue(bitlySchema) + const usage = new Map([['main', new Set(['bitly.clicks'])]]) + const migrations = await generateDatatableMigrations('ws', usage) + const sql = migrations[0].sql + expect(sql).toContain('REFERENCES "bitly"."links" (id)') + expect(sql).not.toMatch(/REFERENCES links\b/) + // The guard skips the ALTER when the constraint already exists, so the name it + // looks up has to be the name the ALTER creates. + const created = sql.match(/ADD CONSTRAINT (\S+)/)?.[1] + expect(created).toBeTruthy() + expect(sql).toContain(`conname = '${created}'`) + }) + it('keeps same-named tables from different schemas both created', async () => { const twoSchemas = { public: {