From 14295998cb25e9db69f72a5a9f5f3c8c900c1a4c Mon Sep 17 00:00:00 2001 From: "Jeremy D. Miller" Date: Thu, 9 Apr 2026 14:49:00 -0500 Subject: [PATCH 1/2] 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) --- .../pk_migration_with_referencing_fks.cs | 148 ++++++++++++++++++ src/Weasel.Postgresql/Tables/TableDelta.cs | 38 +++++ 2 files changed, 186 insertions(+) create mode 100644 src/Weasel.Postgresql.Tests/Tables/pk_migration_with_referencing_fks.cs diff --git a/src/Weasel.Postgresql.Tests/Tables/pk_migration_with_referencing_fks.cs b/src/Weasel.Postgresql.Tests/Tables/pk_migration_with_referencing_fks.cs new file mode 100644 index 00000000..85080e46 --- /dev/null +++ b/src/Weasel.Postgresql.Tests/Tables/pk_migration_with_referencing_fks.cs @@ -0,0 +1,148 @@ +using JasperFx; +using Shouldly; +using Weasel.Core; +using Weasel.Postgresql.Tables; +using Xunit; + +namespace Weasel.Postgresql.Tests.Tables; + +/// +/// Tests for GitHub issue #45: PK columns with FK constraints don't migrate. +/// When a table's primary key is dropped and recreated (CASCADE drops referencing FKs), +/// the migration must recreate the FKs from other tables afterwards. +/// +[Collection("pk_fk_migration")] +public class pk_migration_with_referencing_fks : IntegrationContext +{ + public pk_migration_with_referencing_fks() : base("pk_fk_migration") + { + } + + public override async Task InitializeAsync() + { + await ResetSchema(); + } + + [Fact] + public async Task can_change_pk_columns_when_fk_references_all_pk_columns() + { + // Parent with single-column PK + var parent = new Table(new PostgresqlObjectName(SchemaName, "accounts")); + parent.AddColumn("id").AsPrimaryKey(); + parent.AddColumn("name").NotNull(); + + // Child FK references parent's full PK + var child = new Table(new PostgresqlObjectName(SchemaName, "transactions")); + child.AddColumn("id").AsPrimaryKey(); + child.AddColumn("account_id").NotNull().ForeignKeyTo(parent, "id"); + child.AddColumn("amount"); + + var migrator = new PostgresqlMigrator(); + if (theConnection.State != System.Data.ConnectionState.Open) await theConnection.OpenAsync(); + await theConnection.EnsureSchemaExists(SchemaName); + var migration1 = await SchemaMigration.DetermineAsync(theConnection, CancellationToken.None, parent, child); + await migrator.ApplyAllAsync(theConnection, migration1, AutoCreate.CreateOrUpdate); + + // Now change parent PK to (id, name) — composite + // The child FK stays on (id) alone, so we need a unique constraint on id + // to satisfy PostgreSQL. But the PK change forces a DROP CASCADE + recreate. + var parentV2 = new Table(new PostgresqlObjectName(SchemaName, "accounts")); + parentV2.AddColumn("id").AsPrimaryKey(); + parentV2.AddColumn("name").AsPrimaryKey(); // added to PK + + // Add a unique index on id alone so the FK can still reference it + var uniqueIdx = new IndexDefinition("ix_accounts_id") { IsUnique = true }; + uniqueIdx.Columns = new[] { "id" }; + parentV2.Indexes.Add(uniqueIdx); + + var childV2 = new Table(new PostgresqlObjectName(SchemaName, "transactions")); + childV2.AddColumn("id").AsPrimaryKey(); + childV2.AddColumn("account_id").NotNull().ForeignKeyTo(parentV2, "id"); + childV2.AddColumn("amount"); + + // This should succeed — PK change + FK recreation + var migration2 = await SchemaMigration.DetermineAsync(theConnection, CancellationToken.None, parentV2, childV2); + migration2.Difference.ShouldNotBe(SchemaPatchDifference.None); + await migrator.ApplyAllAsync(theConnection, migration2, AutoCreate.CreateOrUpdate); + + // Verify clean state + var finalMigration = await SchemaMigration.DetermineAsync(theConnection, CancellationToken.None, parentV2, childV2); + finalMigration.Difference.ShouldBe(SchemaPatchDifference.None); + } + + [Fact] + public async Task can_rename_pk_constraint_with_referencing_fk() + { + var parent = new Table(new PostgresqlObjectName(SchemaName, "parent_rename")); + parent.AddColumn("id").AsPrimaryKey(); + parent.AddColumn("name").NotNull(); + parent.PrimaryKeyName = "pk_parent_old"; + + var child = new Table(new PostgresqlObjectName(SchemaName, "child_rename")); + child.AddColumn("id").AsPrimaryKey(); + child.AddColumn("parent_id").NotNull().ForeignKeyTo(parent, "id"); + + var migrator = new PostgresqlMigrator(); + if (theConnection.State != System.Data.ConnectionState.Open) await theConnection.OpenAsync(); + await theConnection.EnsureSchemaExists(SchemaName); + var migration1 = await SchemaMigration.DetermineAsync(theConnection, CancellationToken.None, parent, child); + await migrator.ApplyAllAsync(theConnection, migration1, AutoCreate.CreateOrUpdate); + + // Change only the PK name + var parentV2 = new Table(new PostgresqlObjectName(SchemaName, "parent_rename")); + parentV2.AddColumn("id").AsPrimaryKey(); + parentV2.AddColumn("name").NotNull(); + parentV2.PrimaryKeyName = "pk_parent_new"; + + var childV2 = new Table(new PostgresqlObjectName(SchemaName, "child_rename")); + childV2.AddColumn("id").AsPrimaryKey(); + childV2.AddColumn("parent_id").NotNull().ForeignKeyTo(parentV2, "id"); + + var migration2 = await SchemaMigration.DetermineAsync(theConnection, CancellationToken.None, parentV2, childV2); + await migrator.ApplyAllAsync(theConnection, migration2, AutoCreate.CreateOrUpdate); + + var finalMigration = await SchemaMigration.DetermineAsync(theConnection, CancellationToken.None, parentV2, childV2); + finalMigration.Difference.ShouldBe(SchemaPatchDifference.None); + } + + [Fact] + public async Task ddl_output_includes_fk_recreation_after_pk_change() + { + var parent = new Table(new PostgresqlObjectName(SchemaName, "ddl_parent")); + parent.AddColumn("id").AsPrimaryKey(); + parent.AddColumn("code"); + + var child = new Table(new PostgresqlObjectName(SchemaName, "ddl_child")); + child.AddColumn("id").AsPrimaryKey(); + child.AddColumn("parent_id").NotNull().ForeignKeyTo(parent, "id"); + + var migrator = new PostgresqlMigrator(); + if (theConnection.State != System.Data.ConnectionState.Open) await theConnection.OpenAsync(); + await theConnection.EnsureSchemaExists(SchemaName); + var migration1 = await SchemaMigration.DetermineAsync(theConnection, CancellationToken.None, parent, child); + await migrator.ApplyAllAsync(theConnection, migration1, AutoCreate.CreateOrUpdate); + + // Change PK — keep same column but add unique index so FK still works + var parentV2 = new Table(new PostgresqlObjectName(SchemaName, "ddl_parent")); + parentV2.AddColumn("id").AsPrimaryKey(); + parentV2.AddColumn("code").AsPrimaryKey(); // added to PK + var uniqueIdx = new IndexDefinition("ix_ddl_parent_id") { IsUnique = true }; + uniqueIdx.Columns = new[] { "id" }; + parentV2.Indexes.Add(uniqueIdx); + + var childV2 = new Table(new PostgresqlObjectName(SchemaName, "ddl_child")); + childV2.AddColumn("id").AsPrimaryKey(); + childV2.AddColumn("parent_id").NotNull().ForeignKeyTo(parentV2, "id"); + + var migration2 = await SchemaMigration.DetermineAsync(theConnection, CancellationToken.None, parentV2, childV2); + + var writer = new StringWriter(); + migration2.WriteAllUpdates(writer, migrator, AutoCreate.CreateOrUpdate); + var ddl = writer.ToString(); + + // Should contain: PK drop, PK add, FK recreation + ddl.ShouldContain("drop constraint"); + ddl.ShouldContain("PRIMARY KEY"); + ddl.ShouldContain("FOREIGN KEY"); + } +} diff --git a/src/Weasel.Postgresql/Tables/TableDelta.cs b/src/Weasel.Postgresql/Tables/TableDelta.cs index 19c37f6f..273e8bc4 100644 --- a/src/Weasel.Postgresql/Tables/TableDelta.cs +++ b/src/Weasel.Postgresql/Tables/TableDelta.cs @@ -23,6 +23,13 @@ public TableDelta(Table expected, Table? actual): base(expected, actual) internal ItemDelta ForeignKeys { get; private set; } = null!; + /// + /// Foreign keys from OTHER tables that reference this table's primary key. + /// When the PK changes, these must be dropped and recreated. + /// Populated during . + /// + internal List<(Table OwnerTable, ForeignKey ForeignKey)> ReferencingForeignKeys { get; } = new(); + public SchemaPatchDifference PrimaryKeyDifference { get; private set; } protected override SchemaPatchDifference compare(Table expected, Table? actual) @@ -171,8 +178,17 @@ private void writePrimaryKeyChanges(TextWriter writer) break; } + // CASCADE will also drop FKs from other tables that reference this PK. + // We must recreate those FKs after the PK is altered. writer.WriteLine($"alter table {Expected.Identifier} drop constraint {Actual!.PrimaryKeyName} CASCADE;"); writer.WriteLine($"alter table {Expected.Identifier} add {Expected.PrimaryKeyDeclaration()};"); + + // Recreate foreign keys from other tables that were dropped by CASCADE + foreach (var (ownerTable, fk) in ReferencingForeignKeys) + { + fk.WriteAddStatement(ownerTable, writer); + } + break; case SchemaPatchDifference.Create: @@ -251,6 +267,28 @@ public void PostProcess(IList allDeltas) ForeignKeys = new ItemDelta(Expected.ForeignKeys, Actual.ForeignKeys); Difference = compare(Expected, Actual); } + + // When this table's PK is changing, find FKs from other tables that reference it. + // Those FKs will be dropped by CASCADE and must be recreated after the PK is altered. + if (PrimaryKeyDifference is SchemaPatchDifference.Invalid or SchemaPatchDifference.Update + && Expected.PrimaryKeyColumns.Any()) + { + foreach (var otherDelta in allDeltas.OfType()) + { + if (ReferenceEquals(otherDelta, this)) continue; + + // Check expected FKs that reference this table + foreach (var fk in otherDelta.Expected.ForeignKeys) + { + if (fk.LinkedTable != null && + fk.LinkedTable.QualifiedName.Equals(Expected.Identifier.QualifiedName, + StringComparison.OrdinalIgnoreCase)) + { + ReferencingForeignKeys.Add((otherDelta.Expected, fk)); + } + } + } + } } private void rollbackIndexes(TextWriter writer) From cbccb2fa77582f28cd8c862f6208982b003d2723 Mon Sep 17 00:00:00 2001 From: "Jeremy D. Miller" Date: Thu, 9 Apr 2026 16:04:33 -0500 Subject: [PATCH 2/2] fix: replace ToLower/ToUpper with culture-invariant equivalents for SQL identifiers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In Turkish locale (tr-TR), String.ToLower() converts 'I' to dotless 'ı' (U+0131) instead of 'i', corrupting SQL identifiers (e.g. INFORMATION_SCHEMA → ınformation_schema). Replace all .ToLower()/.ToUpper() calls that operate on SQL identifiers, schema names, column names, type names, and keywords with .ToLowerInvariant()/.ToUpperInvariant(). Adds unit tests that set CultureInfo.CurrentCulture = tr-TR to verify correct normalization. Upstream fix for JasperFx/wolverine#2472 (companion PR to JasperFx/wolverine#2480). Co-Authored-By: Claude Sonnet 4.6 --- src/Weasel.Core/Migrator.cs | 4 ++-- .../Tables/TableColumnTests.cs | 20 +++++++++++++++++++ .../Tables/detecting_table_deltas.cs | 4 ++-- src/Weasel.Postgresql/Extension.cs | 2 +- src/Weasel.Postgresql/PostgresqlProvider.cs | 2 +- .../Tables/IndexDefinition.cs | 2 +- src/Weasel.Postgresql/Tables/TableColumn.cs | 4 ++-- .../Tables/TableColumnTests.cs | 20 +++++++++++++++++++ src/Weasel.SqlServer/SqlServerProvider.cs | 4 ++-- src/Weasel.SqlServer/Tables/Table.cs | 2 +- src/Weasel.SqlServer/Tables/TableColumn.cs | 4 ++-- 11 files changed, 54 insertions(+), 14 deletions(-) diff --git a/src/Weasel.Core/Migrator.cs b/src/Weasel.Core/Migrator.cs index 3c23d538..4f888be0 100644 --- a/src/Weasel.Core/Migrator.cs +++ b/src/Weasel.Core/Migrator.cs @@ -78,13 +78,13 @@ public async Task ReadTemplatesAsync(string directory, CancellationToken ct = de { foreach (var file in FileSystem.FindFiles(directory, FileSet.Shallow("*.function"))) { - var name = Path.GetFileNameWithoutExtension(file).ToLower(); + var name = Path.GetFileNameWithoutExtension(file).ToLowerInvariant(); Templates[name].FunctionCreation = await File.ReadAllTextAsync(file, ct).ConfigureAwait(false); } foreach (var file in FileSystem.FindFiles(directory, FileSet.Shallow("*.table"))) { - var name = Path.GetFileNameWithoutExtension(file).ToLower(); + var name = Path.GetFileNameWithoutExtension(file).ToLowerInvariant(); Templates[name].TableCreation = await File.ReadAllTextAsync(file, ct).ConfigureAwait(false); } diff --git a/src/Weasel.Postgresql.Tests/Tables/TableColumnTests.cs b/src/Weasel.Postgresql.Tests/Tables/TableColumnTests.cs index 0d703e22..0f7cc82e 100644 --- a/src/Weasel.Postgresql.Tests/Tables/TableColumnTests.cs +++ b/src/Weasel.Postgresql.Tests/Tables/TableColumnTests.cs @@ -1,3 +1,4 @@ +using System.Globalization; using NetTopologySuite.Geometries; using Shouldly; using Weasel.Postgresql.Tables; @@ -143,4 +144,23 @@ public void to_function_update_should_quote_reserved_keywords(string keyword) column.ToFunctionUpdate().ShouldBe($"\"{keyword}\" = p_{keyword}"); } + + [Fact] + public void column_name_normalization_is_culture_invariant() + { + // In Turkish locale, "I".ToLower() produces dotless 'ı' (U+0131) instead of 'i', + // which would corrupt SQL identifiers like "INFORMATION_SCHEMA" → "ınformation_schema". + var originalCulture = CultureInfo.CurrentCulture; + try + { + CultureInfo.CurrentCulture = new CultureInfo("tr-TR"); + var column = new TableColumn("INFORMATION_SCHEMA", "VARCHAR"); + column.Name.ShouldBe("information_schema"); + column.Type.ShouldBe("varchar"); + } + finally + { + CultureInfo.CurrentCulture = originalCulture; + } + } } diff --git a/src/Weasel.Postgresql.Tests/Tables/detecting_table_deltas.cs b/src/Weasel.Postgresql.Tests/Tables/detecting_table_deltas.cs index d9d30839..2a840aa5 100644 --- a/src/Weasel.Postgresql.Tests/Tables/detecting_table_deltas.cs +++ b/src/Weasel.Postgresql.Tests/Tables/detecting_table_deltas.cs @@ -67,13 +67,13 @@ public async Task using_reserved_keywords_for_columns() [MemberData(nameof(PostgresReservedKeywords))] public async Task verify_all_postgres_reserved_keywords_work_as_column_names(string keyword) { - var tableName = $"deltas.keyword_{keyword.ToLower()}"; + var tableName = $"deltas.keyword_{keyword.ToLowerInvariant()}"; var table = new Table(tableName); table.AddColumn("id").AsPrimaryKey(); await CreateSchemaObjectInDatabase(table); - table.AddColumn(keyword.ToLower()); + table.AddColumn(keyword.ToLowerInvariant()); await AssertNoDeltasAfterPatching(table); } diff --git a/src/Weasel.Postgresql/Extension.cs b/src/Weasel.Postgresql/Extension.cs index d85032dd..65bea4dd 100644 --- a/src/Weasel.Postgresql/Extension.cs +++ b/src/Weasel.Postgresql/Extension.cs @@ -11,7 +11,7 @@ public class Extension: ISchemaObject { public Extension(string extensionName) { - ExtensionName = extensionName.Trim().ToLower(); + ExtensionName = extensionName.Trim().ToLowerInvariant(); } public string ExtensionName { get; } diff --git a/src/Weasel.Postgresql/PostgresqlProvider.cs b/src/Weasel.Postgresql/PostgresqlProvider.cs index 5940f11c..e3d5a8e1 100644 --- a/src/Weasel.Postgresql/PostgresqlProvider.cs +++ b/src/Weasel.Postgresql/PostgresqlProvider.cs @@ -98,7 +98,7 @@ protected override Type[] determineClrTypesForParameterType(NpgsqlDbType dbType) public string ConvertSynonyms(string type) { - switch (type.ToLower()) + switch (type.ToLowerInvariant()) { case "character varying": case "varchar": diff --git a/src/Weasel.Postgresql/Tables/IndexDefinition.cs b/src/Weasel.Postgresql/Tables/IndexDefinition.cs index 82cf7c68..d0c092a7 100644 --- a/src/Weasel.Postgresql/Tables/IndexDefinition.cs +++ b/src/Weasel.Postgresql/Tables/IndexDefinition.cs @@ -390,7 +390,7 @@ public static IndexDefinition Parse(string definition) while (tokens.Any()) { var current = tokens.Dequeue(); - switch (current.ToUpper()) + switch (current.ToUpperInvariant()) { case "CREATE": case "CONCURRENTLY": diff --git a/src/Weasel.Postgresql/Tables/TableColumn.cs b/src/Weasel.Postgresql/Tables/TableColumn.cs index 4901df04..45c601b1 100644 --- a/src/Weasel.Postgresql/Tables/TableColumn.cs +++ b/src/Weasel.Postgresql/Tables/TableColumn.cs @@ -19,8 +19,8 @@ public TableColumn(string name, string type) throw new ArgumentOutOfRangeException(nameof(type)); } - Name = name.ToLower().Trim().Replace(' ', '_'); - Type = type.ToLower(); + Name = name.ToLowerInvariant().Trim().Replace(' ', '_'); + Type = type.ToLowerInvariant(); } diff --git a/src/Weasel.SqlServer.Tests/Tables/TableColumnTests.cs b/src/Weasel.SqlServer.Tests/Tables/TableColumnTests.cs index e81f7ab8..9bcbb00b 100644 --- a/src/Weasel.SqlServer.Tests/Tables/TableColumnTests.cs +++ b/src/Weasel.SqlServer.Tests/Tables/TableColumnTests.cs @@ -1,3 +1,4 @@ +using System.Globalization; using Shouldly; using Weasel.SqlServer.Tables; using Xunit; @@ -134,4 +135,23 @@ public void change_column_type_sql() table.ColumnFor(columnName)!.AlterColumnTypeSql(table, actualColumn) .ShouldBe($"alter table dbo.people alter column [{columnName}] varchar(200) NOT NULL;"); } + + [Fact] + public void column_name_normalization_is_culture_invariant() + { + // In Turkish locale, "I".ToLower() produces dotless 'ı' (U+0131) instead of 'i', + // which would corrupt SQL identifiers like "INFORMATION_SCHEMA" → "ınformation_schema". + var originalCulture = CultureInfo.CurrentCulture; + try + { + CultureInfo.CurrentCulture = new CultureInfo("tr-TR"); + var column = new TableColumn("INFORMATION_SCHEMA", "VARCHAR"); + column.Name.ShouldBe("information_schema"); + column.Type.ShouldBe("varchar"); + } + finally + { + CultureInfo.CurrentCulture = originalCulture; + } + } } diff --git a/src/Weasel.SqlServer/SqlServerProvider.cs b/src/Weasel.SqlServer/SqlServerProvider.cs index da716af2..30765492 100644 --- a/src/Weasel.SqlServer/SqlServerProvider.cs +++ b/src/Weasel.SqlServer/SqlServerProvider.cs @@ -87,7 +87,7 @@ public override string AddApplicationNameToConnectionString(string connectionStr public string ConvertSynonyms(string type) { - switch (type.ToLower()) + switch (type.ToLowerInvariant()) { case "text": case "varchar": @@ -189,7 +189,7 @@ public override void SetParameterType(SqlParameter parameter, SqlDbType dbType) public static CascadeAction ReadAction(string description) { - switch (description.ToUpper().Trim()) + switch (description.ToUpperInvariant().Trim()) { case "CASCADE": return CascadeAction.Cascade; diff --git a/src/Weasel.SqlServer/Tables/Table.cs b/src/Weasel.SqlServer/Tables/Table.cs index 86ae10aa..3a825e97 100644 --- a/src/Weasel.SqlServer/Tables/Table.cs +++ b/src/Weasel.SqlServer/Tables/Table.cs @@ -128,7 +128,7 @@ public void WriteCreateStatement(Migrator migrator, TextWriter writer) if (migrator.TableCreation == CreationStyle.DropThenCreate) { // drop all FK constraints - var sqlVariableName = $"@sql_{Guid.NewGuid().ToString().ToLower().Replace("-", "_")}"; + var sqlVariableName = $"@sql_{Guid.NewGuid().ToString().ToLowerInvariant().Replace("-", "_")}"; writer.WriteLine("DECLARE {0} NVARCHAR(MAX) = '';", sqlVariableName); writer.WriteLine("SELECT {0} = {1} + 'ALTER TABLE ' + QUOTENAME(OBJECT_SCHEMA_NAME(fk.parent_object_id)) + '.' + QUOTENAME(OBJECT_NAME(fk.parent_object_id)) + ' DROP CONSTRAINT ' + QUOTENAME(fk.name) + ';'", sqlVariableName, sqlVariableName); diff --git a/src/Weasel.SqlServer/Tables/TableColumn.cs b/src/Weasel.SqlServer/Tables/TableColumn.cs index 2a7360ad..4dcd4e8d 100644 --- a/src/Weasel.SqlServer/Tables/TableColumn.cs +++ b/src/Weasel.SqlServer/Tables/TableColumn.cs @@ -18,8 +18,8 @@ public TableColumn(string name, string type) throw new ArgumentOutOfRangeException(nameof(type)); } - Name = name.ToLower().Trim().Replace(' ', '_'); - Type = type.ToLower(); + Name = name.ToLowerInvariant().Trim().Replace(' ', '_'); + Type = type.ToLowerInvariant(); }