mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-07 00:01:49 +00:00
fix: qualify a data table FK target with its schema
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HtYCxXEn2WujwVvh5aZRCa
This commit is contained in:
co-authored by
Claude Opus 5
parent
74137ca12a
commit
69fd71c44f
+17
-2
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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: {
|
||||
|
||||
Reference in New Issue
Block a user