fix(db): idempotent migration script runs clean on SQL Server (#126)

`dotnet ef migrations script --idempotent` puts each migration in ONE batch
behind IF NOT EXISTS guards, and SQL Server compiles the whole batch before any
guard runs. AddAuditLogHashChain adds ChainSeq and then creates two filtered
indexes whose WHERE clause references it in the same batch, so the script
failed with Msg 207 "Invalid column name 'ChainSeq'" on any DB at migration
<= 60 (the 2026-11-11 vd03 deploy hand-inserted GO separators instead).

- AddAuditLogHashChain: the two CREATE INDEX statements go through EXEC(N'...')
  so they compile at execution time. Same indexes, same snapshot; Migrate() /
  database update unaffected; DBs that already ran the migration never run it
  again. The column-level GRANT compiles fine (checked at execution).
- Audit of 58-64: no other migration hits the pattern (the idempotent script
  from 57 now runs clean end to end; AddConfigGraphRowVersions' plain CREATE
  INDEX on a new column is not compile-checked).
- docs/plans/sql/AddAuditLogHashChain.sql: same EXEC wrap. It, and
  PrefixSearchIndexes.sql / AddConfigGraphRowVersions.sql, also carried a
  stray UTF-8 BOM after the prepended header comment that made sqlcmd fail
  with Msg 102 on line 1 of the batch; removed.
- New IdempotentMigrationScriptTests: migrates the fixture DB to 57, runs the
  generated idempotent script batch by batch, asserts every migration applied,
  then re-runs it as a no-op. Fails with Msg 207 without the migration fix.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-11-12 06:55:04 -05:00
parent 424db1555d
commit c7dfc21b4c
5 changed files with 143 additions and 11 deletions
+7 -5
View File
@@ -2,7 +2,9 @@
-- ChainSeq/RowHash on dbo.AuditLog, AuditLogChainHead + AuditLogChainTombstone, two partition-aligned -- ChainSeq/RowHash on dbo.AuditLog, AuditLogChainHead + AuditLogChainTombstone, two partition-aligned
-- filtered indexes, the scadabridge_audit_chainer role. Apply to the central configuration database -- filtered indexes, the scadabridge_audit_chainer role. Apply to the central configuration database
-- with both central nodes stopped (old binaries cannot switch out a partition once the columns exist). -- with both central nodes stopped (old binaries cannot switch out a partition once the columns exist).
BEGIN TRANSACTION; -- The two filtered indexes reference ChainSeq, which this same batch adds; SQL Server compiles a batch
-- before running it, so they are wrapped in EXEC to defer compilation (else Msg 207, ScadaBridge#126).
BEGIN TRANSACTION;
ALTER TABLE [AuditLog] ADD [ChainSeq] bigint NULL; ALTER TABLE [AuditLog] ADD [ChainSeq] bigint NULL;
ALTER TABLE [AuditLog] ADD [RowHash] binary(32) NULL; ALTER TABLE [AuditLog] ADD [RowHash] binary(32) NULL;
@@ -27,16 +29,16 @@ CREATE TABLE [AuditLogChainTombstone] (
); );
CREATE NONCLUSTERED INDEX IX_AuditLog_Unchained EXEC(N'CREATE NONCLUSTERED INDEX IX_AuditLog_Unchained
ON dbo.AuditLog (OccurredAtUtc, EventId) ON dbo.AuditLog (OccurredAtUtc, EventId)
WHERE ChainSeq IS NULL WHERE ChainSeq IS NULL
ON ps_AuditLog_Month(OccurredAtUtc); ON ps_AuditLog_Month(OccurredAtUtc);');
CREATE NONCLUSTERED INDEX IX_AuditLog_ChainSeq EXEC(N'CREATE NONCLUSTERED INDEX IX_AuditLog_ChainSeq
ON dbo.AuditLog (ChainSeq) ON dbo.AuditLog (ChainSeq)
WHERE ChainSeq IS NOT NULL WHERE ChainSeq IS NOT NULL
ON ps_AuditLog_Month(OccurredAtUtc); ON ps_AuditLog_Month(OccurredAtUtc);');
IF DATABASE_PRINCIPAL_ID('scadabridge_audit_chainer') IS NULL IF DATABASE_PRINCIPAL_ID('scadabridge_audit_chainer') IS NULL
+1 -1
View File
@@ -3,7 +3,7 @@
-- MAX(RowVersion)+COUNT_BIG(*) over them is unchanged. Additive: SQL Server fills every existing row as the -- MAX(RowVersion)+COUNT_BIG(*) over them is unchanged. Additive: SQL Server fills every existing row as the
-- column is added; old binaries ignore the column. Apply after AddAuditLogHashChain in the coordinated -- column is added; old binaries ignore the column. Apply after AddAuditLogHashChain in the coordinated
-- central-pair migration window (ScadaBridge#107 item 4). -- central-pair migration window (ScadaBridge#107 item 4).
BEGIN TRANSACTION; BEGIN TRANSACTION;
ALTER TABLE [TemplateScripts] ADD [RowVersion] rowversion NOT NULL; ALTER TABLE [TemplateScripts] ADD [RowVersion] rowversion NOT NULL;
ALTER TABLE [Templates] ADD [RowVersion] rowversion NOT NULL; ALTER TABLE [Templates] ADD [RowVersion] rowversion NOT NULL;
+1 -1
View File
@@ -1,6 +1,6 @@
-- PrefixSearchIndexes (2026-11-09, ScadaBridge#50). Indexes backing the prefix search on the -- PrefixSearchIndexes (2026-11-09, ScadaBridge#50). Indexes backing the prefix search on the
-- Deployments page (DeployedBy, RevisionHash) and the config-audit EntityName filter. -- Deployments page (DeployedBy, RevisionHash) and the config-audit EntityName filter.
BEGIN TRANSACTION; BEGIN TRANSACTION;
ALTER TABLE [TemplateNativeAlarmSources] ADD [IsMesReportable] bit NOT NULL DEFAULT CAST(0 AS bit); ALTER TABLE [TemplateNativeAlarmSources] ADD [IsMesReportable] bit NOT NULL DEFAULT CAST(0 AS bit);
ALTER TABLE [InstanceNativeAlarmSourceOverrides] ADD [IsMesReportableOverride] bit NULL; ALTER TABLE [InstanceNativeAlarmSourceOverrides] ADD [IsMesReportableOverride] bit NULL;
@@ -77,17 +77,24 @@ namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Migrations
// AuditLog is monthly-partitioned with a partition-switch purge, so both indexes // AuditLog is monthly-partitioned with a partition-switch purge, so both indexes
// MUST be partition-aligned (same pattern as ReviewRemediationIndexes). // MUST be partition-aligned (same pattern as ReviewRemediationIndexes).
//
// Both statements reference ChainSeq, which this same migration adds. The
// `dotnet ef migrations script --idempotent` output puts a whole migration in ONE
// batch, and SQL Server compiles the batch before any IF guard runs — so a direct
// reference to the not-yet-existing column fails with Msg 207 (ScadaBridge#126).
// EXEC defers compilation to execution time, after the ADD COLUMN has run. The
// resulting indexes are identical; `database update` / Migrate() are unaffected.
migrationBuilder.Sql(@" migrationBuilder.Sql(@"
CREATE NONCLUSTERED INDEX IX_AuditLog_Unchained EXEC(N'CREATE NONCLUSTERED INDEX IX_AuditLog_Unchained
ON dbo.AuditLog (OccurredAtUtc, EventId) ON dbo.AuditLog (OccurredAtUtc, EventId)
WHERE ChainSeq IS NULL WHERE ChainSeq IS NULL
ON ps_AuditLog_Month(OccurredAtUtc);"); ON ps_AuditLog_Month(OccurredAtUtc);');");
migrationBuilder.Sql(@" migrationBuilder.Sql(@"
CREATE NONCLUSTERED INDEX IX_AuditLog_ChainSeq EXEC(N'CREATE NONCLUSTERED INDEX IX_AuditLog_ChainSeq
ON dbo.AuditLog (ChainSeq) ON dbo.AuditLog (ChainSeq)
WHERE ChainSeq IS NOT NULL WHERE ChainSeq IS NOT NULL
ON ps_AuditLog_Month(OccurredAtUtc);"); ON ps_AuditLog_Month(OccurredAtUtc);');");
migrationBuilder.Sql(@" migrationBuilder.Sql(@"
IF DATABASE_PRINCIPAL_ID('scadabridge_audit_chainer') IS NULL IF DATABASE_PRINCIPAL_ID('scadabridge_audit_chainer') IS NULL
@@ -0,0 +1,123 @@
using System.Text.RegularExpressions;
using Microsoft.Data.SqlClient;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Migrations;
namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests.Migrations;
/// <summary>
/// SQL Server tests for the deploy artifact itself: the <c>dotnet ef migrations script
/// --idempotent</c> output that upgrades a central configuration database (ScadaBridge#126).
/// </summary>
/// <remarks>
/// EF puts each migration of an idempotent script in ONE batch behind per-statement
/// <c>IF NOT EXISTS (… __EFMigrationsHistory …)</c> guards, and SQL Server compiles the whole
/// batch before any guard runs. A raw-SQL statement that references a column added earlier in
/// the same migration therefore fails with Msg 207 even though the guard would have let the
/// ADD COLUMN run first — <c>database update</c> / <c>Migrate()</c> never see it because they
/// send one command at a time. The tests in this class migrate the fixture database down and
/// back up, so they own their fixture and leave the database fully migrated.
/// </remarks>
public sealed class IdempotentMigrationScriptTests : IClassFixture<MsSqlMigrationFixture>
{
// The last migration deployed everywhere before the 2026-11 central-pair migration window
// (migration 57). The idempotent script from here to head is what DEV/QA/prod run.
private const string BaselineMigration = "20260914173147_AddSiteArtifactRevision";
private static readonly Regex BatchSeparator = new(@"^\s*GO\s*$", RegexOptions.Multiline | RegexOptions.IgnoreCase);
private readonly MsSqlMigrationFixture _fixture;
/// <summary>Creates the test class with its own migrated SQL Server database.</summary>
/// <param name="fixture">The per-class migration fixture.</param>
public IdempotentMigrationScriptTests(MsSqlMigrationFixture fixture) => _fixture = fixture;
/// <summary>
/// From the migration-57 baseline the generated idempotent script runs batch by batch
/// without an error, applies every pending migration, and a second run is a no-op.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[SkippableFact]
public async Task IdempotentScript_FromBaseline_RunsClean_AndReRunsAsANoOp()
{
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
await using var ctx = CreateContext();
var migrator = ctx.GetService<IMigrator>();
var all = ctx.Database.GetMigrations().ToList();
var script = migrator.GenerateScript(
fromMigration: BaselineMigration,
options: MigrationsSqlGenerationOptions.Idempotent);
try
{
await migrator.MigrateAsync(BaselineMigration);
Assert.Equal(all.IndexOf(BaselineMigration) + 1, (await ReadHistoryAsync()).Count);
await RunScriptAsync(script);
Assert.Equal(all, await ReadHistoryAsync());
Assert.Equal(2, await CountChainIndexesAsync());
// Second run against the fully-migrated database: every guard is false.
await RunScriptAsync(script);
Assert.Equal(all, await ReadHistoryAsync());
}
finally
{
// Leave the fixture database fully migrated whatever happened above.
await migrator.MigrateAsync();
}
}
private ScadaBridgeDbContext CreateContext()
=> new(new DbContextOptionsBuilder<ScadaBridgeDbContext>()
.UseSqlServer(_fixture.ConnectionString, sql => sql.CommandTimeout(600)).Options);
private async Task RunScriptAsync(string script)
{
await using var conn = new SqlConnection(_fixture.ConnectionString);
await conn.OpenAsync();
foreach (var batch in BatchSeparator.Split(script))
{
if (string.IsNullOrWhiteSpace(batch))
{
continue;
}
await using var cmd = conn.CreateCommand();
cmd.CommandTimeout = 600;
cmd.CommandText = batch;
await cmd.ExecuteNonQueryAsync();
}
}
private async Task<List<string>> ReadHistoryAsync()
{
await using var conn = new SqlConnection(_fixture.ConnectionString);
await conn.OpenAsync();
await using var cmd = conn.CreateCommand();
cmd.CommandText = "SELECT MigrationId FROM __EFMigrationsHistory ORDER BY MigrationId";
var ids = new List<string>();
await using var reader = await cmd.ExecuteReaderAsync();
while (await reader.ReadAsync())
{
ids.Add(reader.GetString(0));
}
return ids;
}
private async Task<int> CountChainIndexesAsync()
{
await using var conn = new SqlConnection(_fixture.ConnectionString);
await conn.OpenAsync();
await using var cmd = conn.CreateCommand();
cmd.CommandText = """
SELECT COUNT(*) FROM sys.indexes
WHERE object_id = OBJECT_ID('dbo.AuditLog')
AND name IN ('IX_AuditLog_Unchained', 'IX_AuditLog_ChainSeq')
""";
return (int)(await cmd.ExecuteScalarAsync())!;
}
}