Skip to content

Commit a9bf41e

Browse files
jeremydmillerclaude
jeremydmiller
and
claude
committed
Fix PK migration when other tables have referencing FKs (#45)
When a table's primary key is dropped and recreated via ALTER TABLE ... DROP CONSTRAINT ... CASCADE, PostgreSQL also drops foreign keys from other tables that reference the PK. Previously these referencing FKs were not recreated, leaving the schema in an inconsistent state. The fix: - In TableDelta.PostProcess(), scan all other table deltas for FKs that reference this table when the PK is being changed - Store those referencing FKs in a new ReferencingForeignKeys list - In writePrimaryKeyChanges(), after recreating the PK, emit ALTER TABLE ... ADD CONSTRAINT for each referencing FK Closes #45 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 0c1e2a1 commit a9bf41e

2 files changed

Lines changed: 186 additions & 0 deletions

File tree

‎src/Weasel.Postgresql.Tests/Tables/pk_migration_with_referencing_fks.cs‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
using JasperFx;
2+
using Shouldly;
3+
using Weasel.Core;
4+
using Weasel.Postgresql.Tables;
5+
using Xunit;
6+
7+
namespace Weasel.Postgresql.Tests.Tables;
8+
9+
/// <summary>
10+
/// Tests for GitHub issue #45: PK columns with FK constraints don't migrate.
11+
/// When a table's primary key is dropped and recreated (CASCADE drops referencing FKs),
12+
/// the migration must recreate the FKs from other tables afterwards.
13+
/// </summary>
14+
[Collection("pk_fk_migration")]
15+
public class pk_migration_with_referencing_fks : IntegrationContext
16+
{
17+
public pk_migration_with_referencing_fks() : base("pk_fk_migration")
18+
{
19+
}
20+
21+
public override async Task InitializeAsync()
22+
{
23+
await ResetSchema();
24+
}
25+
26+
[Fact]
27+
public async Task can_change_pk_columns_when_fk_references_all_pk_columns()
28+
{
29+
// Parent with single-column PK
30+
var parent = new Table(new PostgresqlObjectName(SchemaName, "accounts"));
31+
parent.AddColumn<int>("id").AsPrimaryKey();
32+
parent.AddColumn<string>("name").NotNull();
33+
34+
// Child FK references parent's full PK
35+
var child = new Table(new PostgresqlObjectName(SchemaName, "transactions"));
36+
child.AddColumn<int>("id").AsPrimaryKey();
37+
child.AddColumn<int>("account_id").NotNull().ForeignKeyTo(parent, "id");
38+
child.AddColumn<decimal>("amount");
39+
40+
var migrator = new PostgresqlMigrator();
41+
if (theConnection.State != System.Data.ConnectionState.Open) await theConnection.OpenAsync();
42+
await theConnection.EnsureSchemaExists(SchemaName);
43+
var migration1 = await SchemaMigration.DetermineAsync(theConnection, CancellationToken.None, parent, child);
44+
await migrator.ApplyAllAsync(theConnection, migration1, AutoCreate.CreateOrUpdate);
45+
46+
// Now change parent PK to (id, name) — composite
47+
// The child FK stays on (id) alone, so we need a unique constraint on id
48+
// to satisfy PostgreSQL. But the PK change forces a DROP CASCADE + recreate.
49+
var parentV2 = new Table(new PostgresqlObjectName(SchemaName, "accounts"));
50+
parentV2.AddColumn<int>("id").AsPrimaryKey();
51+
parentV2.AddColumn<string>("name").AsPrimaryKey(); // added to PK
52+
53+
// Add a unique index on id alone so the FK can still reference it
54+
var uniqueIdx = new IndexDefinition("ix_accounts_id") { IsUnique = true };
55+
uniqueIdx.Columns = new[] { "id" };
56+
parentV2.Indexes.Add(uniqueIdx);
57+
58+
var childV2 = new Table(new PostgresqlObjectName(SchemaName, "transactions"));
59+
childV2.AddColumn<int>("id").AsPrimaryKey();
60+
childV2.AddColumn<int>("account_id").NotNull().ForeignKeyTo(parentV2, "id");
61+
childV2.AddColumn<decimal>("amount");
62+
63+
// This should succeed — PK change + FK recreation
64+
var migration2 = await SchemaMigration.DetermineAsync(theConnection, CancellationToken.None, parentV2, childV2);
65+
migration2.Difference.ShouldNotBe(SchemaPatchDifference.None);
66+
await migrator.ApplyAllAsync(theConnection, migration2, AutoCreate.CreateOrUpdate);
67+
68+
// Verify clean state
69+
var finalMigration = await SchemaMigration.DetermineAsync(theConnection, CancellationToken.None, parentV2, childV2);
70+
finalMigration.Difference.ShouldBe(SchemaPatchDifference.None);
71+
}
72+
73+
[Fact]
74+
public async Task can_rename_pk_constraint_with_referencing_fk()
75+
{
76+
var parent = new Table(new PostgresqlObjectName(SchemaName, "parent_rename"));
77+
parent.AddColumn<int>("id").AsPrimaryKey();
78+
parent.AddColumn<string>("name").NotNull();
79+
parent.PrimaryKeyName = "pk_parent_old";
80+
81+
var child = new Table(new PostgresqlObjectName(SchemaName, "child_rename"));
82+
child.AddColumn<int>("id").AsPrimaryKey();
83+
child.AddColumn<int>("parent_id").NotNull().ForeignKeyTo(parent, "id");
84+
85+
var migrator = new PostgresqlMigrator();
86+
if (theConnection.State != System.Data.ConnectionState.Open) await theConnection.OpenAsync();
87+
await theConnection.EnsureSchemaExists(SchemaName);
88+
var migration1 = await SchemaMigration.DetermineAsync(theConnection, CancellationToken.None, parent, child);
89+
await migrator.ApplyAllAsync(theConnection, migration1, AutoCreate.CreateOrUpdate);
90+
91+
// Change only the PK name
92+
var parentV2 = new Table(new PostgresqlObjectName(SchemaName, "parent_rename"));
93+
parentV2.AddColumn<int>("id").AsPrimaryKey();
94+
parentV2.AddColumn<string>("name").NotNull();
95+
parentV2.PrimaryKeyName = "pk_parent_new";
96+
97+
var childV2 = new Table(new PostgresqlObjectName(SchemaName, "child_rename"));
98+
childV2.AddColumn<int>("id").AsPrimaryKey();
99+
childV2.AddColumn<int>("parent_id").NotNull().ForeignKeyTo(parentV2, "id");
100+
101+
var migration2 = await SchemaMigration.DetermineAsync(theConnection, CancellationToken.None, parentV2, childV2);
102+
await migrator.ApplyAllAsync(theConnection, migration2, AutoCreate.CreateOrUpdate);
103+
104+
var finalMigration = await SchemaMigration.DetermineAsync(theConnection, CancellationToken.None, parentV2, childV2);
105+
finalMigration.Difference.ShouldBe(SchemaPatchDifference.None);
106+
}
107+
108+
[Fact]
109+
public async Task ddl_output_includes_fk_recreation_after_pk_change()
110+
{
111+
var parent = new Table(new PostgresqlObjectName(SchemaName, "ddl_parent"));
112+
parent.AddColumn<int>("id").AsPrimaryKey();
113+
parent.AddColumn<string>("code");
114+
115+
var child = new Table(new PostgresqlObjectName(SchemaName, "ddl_child"));
116+
child.AddColumn<int>("id").AsPrimaryKey();
117+
child.AddColumn<int>("parent_id").NotNull().ForeignKeyTo(parent, "id");
118+
119+
var migrator = new PostgresqlMigrator();
120+
if (theConnection.State != System.Data.ConnectionState.Open) await theConnection.OpenAsync();
121+
await theConnection.EnsureSchemaExists(SchemaName);
122+
var migration1 = await SchemaMigration.DetermineAsync(theConnection, CancellationToken.None, parent, child);
123+
await migrator.ApplyAllAsync(theConnection, migration1, AutoCreate.CreateOrUpdate);
124+
125+
// Change PK — keep same column but add unique index so FK still works
126+
var parentV2 = new Table(new PostgresqlObjectName(SchemaName, "ddl_parent"));
127+
parentV2.AddColumn<int>("id").AsPrimaryKey();
128+
parentV2.AddColumn<string>("code").AsPrimaryKey(); // added to PK
129+
var uniqueIdx = new IndexDefinition("ix_ddl_parent_id") { IsUnique = true };
130+
uniqueIdx.Columns = new[] { "id" };
131+
parentV2.Indexes.Add(uniqueIdx);
132+
133+
var childV2 = new Table(new PostgresqlObjectName(SchemaName, "ddl_child"));
134+
childV2.AddColumn<int>("id").AsPrimaryKey();
135+
childV2.AddColumn<int>("parent_id").NotNull().ForeignKeyTo(parentV2, "id");
136+
137+
var migration2 = await SchemaMigration.DetermineAsync(theConnection, CancellationToken.None, parentV2, childV2);
138+
139+
var writer = new StringWriter();
140+
migration2.WriteAllUpdates(writer, migrator, AutoCreate.CreateOrUpdate);
141+
var ddl = writer.ToString();
142+
143+
// Should contain: PK drop, PK add, FK recreation
144+
ddl.ShouldContain("drop constraint");
145+
ddl.ShouldContain("PRIMARY KEY");
146+
ddl.ShouldContain("FOREIGN KEY");
147+
}
148+
}

‎src/Weasel.Postgresql/Tables/TableDelta.cs‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,13 @@ public TableDelta(Table expected, Table? actual): base(expected, actual)
2323

2424
internal ItemDelta<ForeignKey> ForeignKeys { get; private set; } = null!;
2525

26+
/// <summary>
27+
/// Foreign keys from OTHER tables that reference this table's primary key.
28+
/// When the PK changes, these must be dropped and recreated.
29+
/// Populated during <see cref="PostProcess" />.
30+
/// </summary>
31+
internal List<(Table OwnerTable, ForeignKey ForeignKey)> ReferencingForeignKeys { get; } = new();
32+
2633
public SchemaPatchDifference PrimaryKeyDifference { get; private set; }
2734

2835
protected override SchemaPatchDifference compare(Table expected, Table? actual)
@@ -171,8 +178,17 @@ private void writePrimaryKeyChanges(TextWriter writer)
171178
break;
172179
}
173180

181+
// CASCADE will also drop FKs from other tables that reference this PK.
182+
// We must recreate those FKs after the PK is altered.
174183
writer.WriteLine($"alter table {Expected.Identifier} drop constraint {Actual!.PrimaryKeyName} CASCADE;");
175184
writer.WriteLine($"alter table {Expected.Identifier} add {Expected.PrimaryKeyDeclaration()};");
185+
186+
// Recreate foreign keys from other tables that were dropped by CASCADE
187+
foreach (var (ownerTable, fk) in ReferencingForeignKeys)
188+
{
189+
fk.WriteAddStatement(ownerTable, writer);
190+
}
191+
176192
break;
177193

178194
case SchemaPatchDifference.Create:
@@ -251,6 +267,28 @@ public void PostProcess(IList<ISchemaObjectDelta> allDeltas)
251267
ForeignKeys = new ItemDelta<ForeignKey>(Expected.ForeignKeys, Actual.ForeignKeys);
252268
Difference = compare(Expected, Actual);
253269
}
270+
271+
// When this table's PK is changing, find FKs from other tables that reference it.
272+
// Those FKs will be dropped by CASCADE and must be recreated after the PK is altered.
273+
if (PrimaryKeyDifference is SchemaPatchDifference.Invalid or SchemaPatchDifference.Update
274+
&& Expected.PrimaryKeyColumns.Any())
275+
{
276+
foreach (var otherDelta in allDeltas.OfType<TableDelta>())
277+
{
278+
if (ReferenceEquals(otherDelta, this)) continue;
279+
280+
// Check expected FKs that reference this table
281+
foreach (var fk in otherDelta.Expected.ForeignKeys)
282+
{
283+
if (fk.LinkedTable != null &&
284+
fk.LinkedTable.QualifiedName.Equals(Expected.Identifier.QualifiedName,
285+
StringComparison.OrdinalIgnoreCase))
286+
{
287+
ReferencingForeignKeys.Add((otherDelta.Expected, fk));
288+
}
289+
}
290+
}
291+
}
254292
}
255293

256294
private void rollbackIndexes(TextWriter writer)

0 commit comments

Comments
 (0)