From a30a742d267172b67b62c9e628a3de2d85497449 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 29 Sep 2026 04:12:37 -0400 Subject: [PATCH 1/4] Lite refuses to open a data file stamped with a newer schema version than the app's (#4727) An older Lite on the shared data root opened a file whose schema had moved on, and every batch for the 13 tables that gained columns in versions 60 to 65 then failed with only a log line to show for it. The stamp is now read before anything is created, and a newer one throws SchemaVersionTooNewException naming both versions and the file, which the startup handler shows. The file is not written to. --- Lite.Tests/NewerSchemaFileGuardTests.cs | 97 +++++++++++++++++++ Lite/Database/DuckDbInitializer.cs | 15 ++- Lite/Database/SchemaVersionTooNewException.cs | 38 ++++++++ 3 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 Lite.Tests/NewerSchemaFileGuardTests.cs create mode 100644 Lite/Database/SchemaVersionTooNewException.cs diff --git a/Lite.Tests/NewerSchemaFileGuardTests.cs b/Lite.Tests/NewerSchemaFileGuardTests.cs new file mode 100644 index 0000000000..1660296931 --- /dev/null +++ b/Lite.Tests/NewerSchemaFileGuardTests.cs @@ -0,0 +1,97 @@ +using System; +using System.IO; +using System.Security.Cryptography; +using System.Threading.Tasks; +using DuckDB.NET.Data; +using PerformanceMonitorLite.Database; +using Xunit; + +namespace PerformanceMonitorLite.Tests; + +/// +/// #4727: a data file stamped with a schema version newer than the app's is refused, with a message naming both +/// versions, and nothing is written to it. A read-only open would not be enough: the collectors would still fail. +/// +public class NewerSchemaFileGuardTests : IDisposable +{ + private readonly string _tempDir; + private readonly string _dbPath; + + public NewerSchemaFileGuardTests() + { + _tempDir = Path.Combine(Path.GetTempPath(), "LiteTests_" + Guid.NewGuid().ToString("N")[..8]); + Directory.CreateDirectory(_tempDir); + _dbPath = Path.Combine(_tempDir, "test.duckdb"); + } + + public void Dispose() + { + try + { + if (Directory.Exists(_tempDir)) + Directory.Delete(_tempDir, recursive: true); + } + catch + { + /* Best-effort cleanup */ + } + } + + [Fact] + public async Task AFileStampedOneVersionAboveTheApp_IsRefused_AndNothingIsWrittenToIt() + { + var newer = DuckDbInitializer.CurrentSchemaVersion + 1; + + using (var seed = new DuckDBConnection($"Data Source={_dbPath}")) + { + await seed.OpenAsync(); + await ExecAsync(seed, "CREATE TABLE schema_version (version INTEGER NOT NULL)"); + await ExecAsync(seed, $"INSERT INTO schema_version VALUES ({newer})"); + } + + var bytesBefore = SHA256.HashData(File.ReadAllBytes(_dbPath)); + var writtenBefore = File.GetLastWriteTimeUtc(_dbPath); + + var initializer = new DuckDbInitializer(_dbPath); + var refusal = await Assert.ThrowsAsync(() => initializer.InitializeAsync()); + + Assert.Equal(newer, refusal.FileVersion); + Assert.Equal(DuckDbInitializer.CurrentSchemaVersion, refusal.AppVersion); + Assert.Contains($"schema version {newer}", refusal.Message, StringComparison.Ordinal); + Assert.Contains($"schema version {DuckDbInitializer.CurrentSchemaVersion}", refusal.Message, StringComparison.Ordinal); + + Assert.Equal(bytesBefore, SHA256.HashData(File.ReadAllBytes(_dbPath))); + Assert.Equal(writtenBefore, File.GetLastWriteTimeUtc(_dbPath)); + Assert.False(File.Exists(_dbPath + ".wal"), "a refused open must not leave a write-ahead log behind"); + + /* And no table was created: the stamp table is still the only thing in the file. */ + using var verify = new DuckDBConnection($"Data Source={_dbPath};ACCESS_MODE=READ_ONLY"); + await verify.OpenAsync(); + using var count = verify.CreateCommand(); + count.CommandText = "SELECT COUNT(*) FROM information_schema.tables WHERE table_schema = 'main'"; + Assert.Equal(1L, Convert.ToInt64(await count.ExecuteScalarAsync())); + } + + [Fact] + public async Task AFileStampedAtTheAppsOwnVersion_StillOpens() + { + var first = new DuckDbInitializer(_dbPath); + await first.InitializeAsync(); + + var second = new DuckDbInitializer(_dbPath); + await second.InitializeAsync(); + + using var verify = new DuckDBConnection($"Data Source={_dbPath}"); + await verify.OpenAsync(); + using var version = verify.CreateCommand(); + version.CommandText = "SELECT MAX(version) FROM schema_version"; + Assert.Equal((long)DuckDbInitializer.CurrentSchemaVersion, Convert.ToInt64(await version.ExecuteScalarAsync())); + } + + private static async Task ExecAsync(DuckDBConnection connection, string sql) + { + using var command = connection.CreateCommand(); + command.CommandText = sql; + await command.ExecuteNonQueryAsync(); + } +} diff --git a/Lite/Database/DuckDbInitializer.cs b/Lite/Database/DuckDbInitializer.cs index 3c4af710e5..1669406731 100644 --- a/Lite/Database/DuckDbInitializer.cs +++ b/Lite/Database/DuckDbInitializer.cs @@ -781,11 +781,22 @@ private async Task InitializeCoreAsync() using (connection) { + /* #4727: read the stamp BEFORE anything is created, and refuse a file stamped newer than this build + (an older Lite on the shared data root after a rollback). Versions 60 to 65 added columns to 13 + tables, so carrying on would fail every batch for them with only a log line to show for it. A + missing schema_version table reads as version 0 here, so a fresh file still takes the create path. */ + var existingVersion = await GetSchemaVersionAsync(connection); + if (existingVersion > CurrentSchemaVersion) + { + _logger?.LogError( + "Refusing to open {Path}: schema v{FileVersion} is newer than this build's v{AppVersion}", + _databasePath, existingVersion, CurrentSchemaVersion); + throw new SchemaVersionTooNewException(_databasePath, existingVersion, CurrentSchemaVersion); + } + await ExecuteNonQueryAsync(connection, "CREATE TABLE IF NOT EXISTS schema_version (version INTEGER NOT NULL)"); - var existingVersion = await GetSchemaVersionAsync(connection); - /* On a fresh/reset database (v0), skip migrations entirely — they DROP tables expecting CREATE TABLE to follow, which is destructive on a blank DB. Just create tables with the current schema and stamp the version. */ diff --git a/Lite/Database/SchemaVersionTooNewException.cs b/Lite/Database/SchemaVersionTooNewException.cs new file mode 100644 index 0000000000..bb32981bbc --- /dev/null +++ b/Lite/Database/SchemaVersionTooNewException.cs @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; + +namespace PerformanceMonitorLite.Database; + +/// +/// #4727: the data file is stamped with a schema version newer than this build understands (an older Lite pointed +/// at the shared data root after a rollback, or an older portable copy). Schema versions 60 to 65 added columns to +/// 13 tables, so an older build that carried on would fail every batch for those tables with only a log line to +/// show for it. throws this before it creates or alters anything, +/// and the startup handler in MainWindow shows to the user. +/// +public sealed class SchemaVersionTooNewException : InvalidOperationException +{ + public SchemaVersionTooNewException(string databasePath, int fileVersion, int appVersion) + : base( + $"The data file {databasePath} was written by a newer version of Performance Monitor Lite. " + + $"It is at schema version {fileVersion}, and this version only understands up to schema version {appVersion}. " + + "The file has not been changed. Update Performance Monitor Lite to the newer version, " + + "or start this one against a different data folder.") + { + FileVersion = fileVersion; + AppVersion = appVersion; + } + + /// The schema version the data file is stamped with. + public int FileVersion { get; } + + /// The newest schema version this build understands. + public int AppVersion { get; } +} From 111da1d9342c4e0850213500e44eb689e45fee9d Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 29 Sep 2026 05:11:11 -0400 Subject: [PATCH 2/4] Lite re-applies the columns versions 60 to 65 added on every start, so a failed or skipped add is healed (#4727) The v60 to v65 steps each caught a failed ALTER, logged a warning and let the version be stamped, so the step never ran again and that table's batches failed on every start. The 15 (table, column, type) entries are now one list, AddedColumns. Each step adds its own version's entries through one shared method, and the same method runs over the whole list on every start of an existing file (existing version above 0), after the table and index statements. It reads which tables and columns exist in one query, runs ADD COLUMN IF NOT EXISTS only for a missing column, skips a table that does not exist yet (the table statements create it with the column), and logs one Error naming the table and column for an add that still fails, so the start continues and the next start retries it. ADD COLUMN goes through on a table that has an index and a v_ view (only DROP COLUMN and ALTER COLUMN hit the Dependency Error), so no index is dropped first. The stamp is not held back. The source pins for the v60 to v64 steps now assert the entries in the list and the step's call to the shared method instead of the literal ALTER text they no longer contain. --- Lite.Tests/DeltaFamilyIntervalColumnTests.cs | 33 +-- Lite.Tests/PerfmonCounterTypeTests.cs | 6 +- Lite.Tests/QsCaptureModeRungTests.cs | 7 +- Lite.Tests/SchemaColumnHealingTests.cs | 199 +++++++++++++++++++ Lite.Tests/TimeHonestyRungTests.cs | 7 +- Lite/Database/DuckDbInitializer.cs | 189 ++++++++++-------- 6 files changed, 339 insertions(+), 102 deletions(-) create mode 100644 Lite.Tests/SchemaColumnHealingTests.cs diff --git a/Lite.Tests/DeltaFamilyIntervalColumnTests.cs b/Lite.Tests/DeltaFamilyIntervalColumnTests.cs index c83dfcc487..599a8dc80a 100644 --- a/Lite.Tests/DeltaFamilyIntervalColumnTests.cs +++ b/Lite.Tests/DeltaFamilyIntervalColumnTests.cs @@ -184,12 +184,16 @@ public void TheLiteMigration_AddsTheColumnToAllFour_AtSchemaVersion60() var source = RepoSource.Read("Lite", "Database", "DuckDbInitializer.cs"); Assert.Contains("if (fromVersion < 60)", source, StringComparison.Ordinal); - Assert.Contains( - "foreach (var table in new[] { \"wait_stats\", \"file_io_stats\", \"latch_stats\", \"spinlock_stats\" })", - source, StringComparison.Ordinal); - Assert.Contains( - "$\"ALTER TABLE {table} ADD COLUMN IF NOT EXISTS sample_interval_seconds INTEGER\"", - source, StringComparison.Ordinal); + + /* #4727: the entries moved out of the step into DuckDbInitializer.AddedColumns; the step adds its own + version's entries through the shared AddMissingColumnsAsync, one idempotent ADD COLUMN per entry. */ + Assert.Contains("AddMissingColumnsAsync(connection, AddedColumnsForVersion(60))", source, StringComparison.Ordinal); + foreach (var table in new[] { "wait_stats", "file_io_stats", "latch_stats", "spinlock_stats" }) + { + Assert.Contains($"(60, \"{table}\", \"sample_interval_seconds\", \"INTEGER\")", source, StringComparison.Ordinal); + } + + Assert.Contains("ADD COLUMN IF NOT EXISTS {column} {type}", source, StringComparison.Ordinal); } /// @@ -207,18 +211,21 @@ public void TheLiteMigration_CompletesTheIntervalAndStoresTheOffsets_AtSchemaVer var source = RepoSource.Read("Lite", "Database", "DuckDbInitializer.cs"); var block = source[source.IndexOf("if (fromVersion < 61)", StringComparison.Ordinal)..]; - foreach (var pair in new[] + /* #4727: the four entries live in DuckDbInitializer.AddedColumns (INTEGER each); the step adds its own + version's entries through the shared AddMissingColumnsAsync. */ + foreach (var entry in new[] { - "(\"procedure_stats\", \"sample_interval_seconds\")", - "(\"memory_grant_stats\", \"sample_interval_seconds\")", - "(\"query_stats\", \"statement_start_offset\")", - "(\"query_stats\", \"statement_end_offset\")", + "(61, \"procedure_stats\", \"sample_interval_seconds\", \"INTEGER\")", + "(61, \"memory_grant_stats\", \"sample_interval_seconds\", \"INTEGER\")", + "(61, \"query_stats\", \"statement_start_offset\", \"INTEGER\")", + "(61, \"query_stats\", \"statement_end_offset\", \"INTEGER\")", }) { - Assert.Contains(pair, block, StringComparison.Ordinal); + Assert.Contains(entry, block, StringComparison.Ordinal); } - Assert.Contains("$\"ALTER TABLE {table} ADD COLUMN IF NOT EXISTS {column} INTEGER\"", block, StringComparison.Ordinal); + Assert.Contains("AddMissingColumnsAsync(connection, AddedColumnsForVersion(61))", block, StringComparison.Ordinal); + Assert.Contains("ADD COLUMN IF NOT EXISTS {column} {type}", block, StringComparison.Ordinal); /* The offsets' semantics, in the block, in these words: the pair a future reader second-guesses. */ Assert.Contains("BYTES", block, StringComparison.Ordinal); diff --git a/Lite.Tests/PerfmonCounterTypeTests.cs b/Lite.Tests/PerfmonCounterTypeTests.cs index 150f9e57c1..869f230a4c 100644 --- a/Lite.Tests/PerfmonCounterTypeTests.cs +++ b/Lite.Tests/PerfmonCounterTypeTests.cs @@ -128,7 +128,11 @@ public void TheLiteMigration_StoresTheCounterType_AtSchemaVersion62() Assert.True(start >= 0, "DuckDbInitializer has no v62 block"); var block = source[start..]; - Assert.Contains("\"ALTER TABLE perfmon_stats ADD COLUMN IF NOT EXISTS cntr_type INTEGER\"", block, StringComparison.Ordinal); + /* #4727: the entry lives in DuckDbInitializer.AddedColumns; the step adds it through the shared + AddMissingColumnsAsync, one idempotent ADD COLUMN IF NOT EXISTS. */ + Assert.Contains("(62, \"perfmon_stats\", \"cntr_type\", \"INTEGER\")", block, StringComparison.Ordinal); + Assert.Contains("AddMissingColumnsAsync(connection, AddedColumnsForVersion(62))", block, StringComparison.Ordinal); + Assert.Contains("ADD COLUMN IF NOT EXISTS {column} {type}", block, StringComparison.Ordinal); Assert.Contains("Running migration to v62", block, StringComparison.Ordinal); foreach (var phrase in new[] { "COUNTS", "LEVELS", "falling level", "counter reset", "NULL delta and NULL interval", "name-suffix proxy", "Appended at the end", "Nothing to backfill", "REQUIRED on this side" }) { diff --git a/Lite.Tests/QsCaptureModeRungTests.cs b/Lite.Tests/QsCaptureModeRungTests.cs index 8fe66123da..d30c9d625e 100644 --- a/Lite.Tests/QsCaptureModeRungTests.cs +++ b/Lite.Tests/QsCaptureModeRungTests.cs @@ -61,8 +61,11 @@ public void TheLiteMigration_StoresBothCaptureModes_AtSchemaVersion64() Assert.True(start >= 0, "DuckDbInitializer has no v64 block"); var block = source[start..]; - Assert.Contains("(\"query_store_health\", \"query_capture_mode\", \"VARCHAR\")", block, StringComparison.Ordinal); - Assert.Contains("(\"query_store_health\", \"wait_stats_capture_mode\", \"VARCHAR\")", block, StringComparison.Ordinal); + /* #4727: the entries live in DuckDbInitializer.AddedColumns; the step adds its own version's entries through + the shared AddMissingColumnsAsync. */ + Assert.Contains("(64, \"query_store_health\", \"query_capture_mode\", \"VARCHAR\")", block, StringComparison.Ordinal); + Assert.Contains("(64, \"query_store_health\", \"wait_stats_capture_mode\", \"VARCHAR\")", block, StringComparison.Ordinal); + Assert.Contains("AddMissingColumnsAsync(connection, AddedColumnsForVersion(64))", block, StringComparison.Ordinal); Assert.Contains("ADD COLUMN IF NOT EXISTS {column} {type}", block, StringComparison.Ordinal); Assert.Contains("Running migration to v64", block, StringComparison.Ordinal); foreach (var phrase in new[] diff --git a/Lite.Tests/SchemaColumnHealingTests.cs b/Lite.Tests/SchemaColumnHealingTests.cs new file mode 100644 index 0000000000..6f61172550 --- /dev/null +++ b/Lite.Tests/SchemaColumnHealingTests.cs @@ -0,0 +1,199 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using DuckDB.NET.Data; +using Microsoft.Extensions.Logging; +using PerformanceMonitorLite.Database; +using Xunit; + +namespace PerformanceMonitorLite.Tests; + +/// +/// #4727 item 2: the columns that schema versions 60 to 65 add are re-applied on every start of an existing +/// data file, so a file where one failed to apply (or that was stamped without it) gets it back instead of +/// failing every batch for that table. The stamp is not held back: the heal does not depend on it. +/// +public sealed class SchemaColumnHealingTests : IDisposable +{ + /// The 15 columns versions 60 to 65 add, spelled out here on purpose so a trimmed production list fails. + private static readonly (string Table, string Column, string Type)[] NewerColumns = + { + ("wait_stats", "sample_interval_seconds", "INTEGER"), + ("file_io_stats", "sample_interval_seconds", "INTEGER"), + ("latch_stats", "sample_interval_seconds", "INTEGER"), + ("spinlock_stats", "sample_interval_seconds", "INTEGER"), + ("procedure_stats", "sample_interval_seconds", "INTEGER"), + ("memory_grant_stats", "sample_interval_seconds", "INTEGER"), + ("query_stats", "statement_start_offset", "INTEGER"), + ("query_stats", "statement_end_offset", "INTEGER"), + ("perfmon_stats", "cntr_type", "INTEGER"), + ("cpu_utilization_stats", "sample_time_utc", "TIMESTAMP"), + ("server_properties", "time_zone_id", "VARCHAR"), + ("query_store_health", "query_capture_mode", "VARCHAR"), + ("query_store_health", "wait_stats_capture_mode", "VARCHAR"), + ("ag_replica_states", "group_id", "VARCHAR"), + ("ag_database_replica_states", "group_id", "VARCHAR"), + }; + + private readonly string _tempDir; + private readonly string _dbPath; + + public SchemaColumnHealingTests() + { + _tempDir = Path.Combine(Path.GetTempPath(), "SchemaColumnHealingTests_" + Guid.NewGuid().ToString("N")[..8]); + Directory.CreateDirectory(_tempDir); + _dbPath = Path.Combine(_tempDir, "lite.duckdb"); + } + + public void Dispose() + { + try { if (Directory.Exists(_tempDir)) Directory.Delete(_tempDir, recursive: true); } + catch { /* best-effort cleanup */ } + } + + [Fact] + public async Task AFileStampedCurrent_ButMissingTheNewerColumns_GetsEveryOneBackOnTheNextStart() + { + using (var first = new DuckDbInitializer(_dbPath)) + { + await first.InitializeAsync(); + } + + using (var conn = new DuckDBConnection($"Data Source={_dbPath}")) + { + await conn.OpenAsync(); + + /* DuckDB refuses DROP COLUMN on a table that has an index (a Dependency Error); a view over the table + does not block it. So only the indexes go, and they are put straight back with the columns gone: + the file then looks the way a real one does when a column add failed, with its indexes and its v_ + views all standing and the stamp at the current version. */ + foreach (var table in NewerColumns.Select(c => c.Table).Distinct()) + { + foreach (var index in await ListAsync(conn, $"SELECT index_name FROM duckdb_indexes() WHERE table_name = '{table}'")) + await ExecAsync(conn, $"DROP INDEX IF EXISTS {index}"); + foreach (var (_, column, _) in NewerColumns.Where(c => c.Table == table)) + await ExecAsync(conn, $"ALTER TABLE {table} DROP COLUMN {column}"); + } + + foreach (var statement in Schema.GetAllIndexStatements()) + await ExecAsync(conn, statement); + + /* The arrange step holds: every column gone, every index and view back, the stamp current. */ + foreach (var (table, column, _) in NewerColumns) + { + Assert.Equal(0L, await CountAsync(conn, $"SELECT COUNT(*) FROM information_schema.columns WHERE table_name = '{table}' AND column_name = '{column}'")); + Assert.True(await CountAsync(conn, $"SELECT COUNT(*) FROM duckdb_indexes() WHERE table_name = '{table}'") >= 1, $"index on {table}"); + Assert.Equal(1L, await CountAsync(conn, $"SELECT COUNT(*) FROM duckdb_views() WHERE NOT internal AND view_name = 'v_{table}'")); + } + + Assert.Equal((long)DuckDbInitializer.CurrentSchemaVersion, await CountAsync(conn, "SELECT MAX(version) FROM schema_version")); + } + + using (var second = new DuckDbInitializer(_dbPath)) + { + await second.InitializeAsync(); + } + + using (var conn = new DuckDBConnection($"Data Source={_dbPath}")) + { + await conn.OpenAsync(); + foreach (var (table, column, type) in NewerColumns) + { + var types = await ListAsync(conn, $"SELECT data_type FROM information_schema.columns WHERE table_name = '{table}' AND column_name = '{column}'"); + Assert.True(types.Count == 1, $"{table}.{column} is back"); + Assert.Equal(type, types[0]); + + /* The passthrough view reads the restored column too (Lite rebuilds every v_ view on start). */ + Assert.Contains(column, await ListAsync(conn, $"SELECT column_name FROM (DESCRIBE SELECT * FROM v_{table})")); + } + + Assert.Equal((long)DuckDbInitializer.CurrentSchemaVersion, await CountAsync(conn, "SELECT MAX(version) FROM schema_version")); + } + } + + /// + /// A column add that still fails logs ONE Error naming the table and column, and the loop moves on to the + /// next entry, so the start continues and the next start retries it. A type DuckDB does not know makes the + /// ALTER itself fail on a table that exists. + /// + [Fact] + public async Task AColumnAddThatFails_LogsOneErrorNamingTheColumn_AndTheNextEntryIsStillAdded() + { + var log = new CapturingLogger(); + using var initializer = new DuckDbInitializer(_dbPath, log); + await initializer.InitializeAsync(); + log.Entries.Clear(); + + using var conn = new DuckDBConnection($"Data Source={_dbPath}"); + await conn.OpenAsync(); + + await initializer.AddMissingColumnsAsync(conn, new[] + { + (0, "wait_stats", "broken_column", "NOT_A_TYPE"), + (0, "wait_stats", "healthy_column", "INTEGER"), + }); + + var error = Assert.Single(log.Entries, e => e.Level == LogLevel.Error); + Assert.Contains("wait_stats.broken_column", error.Message, StringComparison.Ordinal); + Assert.Equal(0L, await CountAsync(conn, "SELECT COUNT(*) FROM information_schema.columns WHERE table_name = 'wait_stats' AND column_name = 'broken_column'")); + Assert.Equal(1L, await CountAsync(conn, "SELECT COUNT(*) FROM information_schema.columns WHERE table_name = 'wait_stats' AND column_name = 'healthy_column'")); + } + + /// + /// A file older than a table has no such table when a migration step runs (the table statements after the + /// steps create it, with the column). That is not a failure and must not read as one in the log. + /// + [Fact] + public async Task ATableThatDoesNotExistYet_IsSkippedWithoutAWarningOrAnError() + { + var log = new CapturingLogger(); + using var initializer = new DuckDbInitializer(_dbPath, log); + await initializer.InitializeAsync(); + log.Entries.Clear(); + + using var conn = new DuckDBConnection($"Data Source={_dbPath}"); + await conn.OpenAsync(); + + await initializer.AddMissingColumnsAsync(conn, new[] { (0, "table_created_later", "some_column", "INTEGER") }); + + Assert.DoesNotContain(log.Entries, e => e.Level >= LogLevel.Warning); + } + + private sealed class CapturingLogger : ILogger + { + public List<(LogLevel Level, string Message)> Entries { get; } = new(); + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) => Entries.Add((logLevel, formatter(state, exception))); + } + + private static async Task ExecAsync(DuckDBConnection conn, string sql) + { + using var cmd = conn.CreateCommand(); + cmd.CommandText = sql; + await cmd.ExecuteNonQueryAsync(); + } + + private static async Task CountAsync(DuckDBConnection conn, string sql) + { + using var cmd = conn.CreateCommand(); + cmd.CommandText = sql; + return Convert.ToInt64(await cmd.ExecuteScalarAsync()); + } + + private static async Task> ListAsync(DuckDBConnection conn, string sql) + { + var rows = new List(); + using var cmd = conn.CreateCommand(); + cmd.CommandText = sql; + using var reader = await cmd.ExecuteReaderAsync(); + while (await reader.ReadAsync()) rows.Add(reader.GetValue(0)?.ToString() ?? ""); + return rows; + } +} diff --git a/Lite.Tests/TimeHonestyRungTests.cs b/Lite.Tests/TimeHonestyRungTests.cs index 93a4999de7..80de5540d1 100644 --- a/Lite.Tests/TimeHonestyRungTests.cs +++ b/Lite.Tests/TimeHonestyRungTests.cs @@ -59,8 +59,11 @@ public void TheLiteMigration_StoresBothColumns_AtSchemaVersion63() Assert.True(start >= 0, "DuckDbInitializer has no v63 block"); var block = source[start..]; - Assert.Contains("(\"cpu_utilization_stats\", \"sample_time_utc\", \"TIMESTAMP\")", block, StringComparison.Ordinal); - Assert.Contains("(\"server_properties\", \"time_zone_id\", \"VARCHAR\")", block, StringComparison.Ordinal); + /* #4727: the entries live in DuckDbInitializer.AddedColumns; the step adds its own version's entries through + the shared AddMissingColumnsAsync. */ + Assert.Contains("(63, \"cpu_utilization_stats\", \"sample_time_utc\", \"TIMESTAMP\")", block, StringComparison.Ordinal); + Assert.Contains("(63, \"server_properties\", \"time_zone_id\", \"VARCHAR\")", block, StringComparison.Ordinal); + Assert.Contains("AddMissingColumnsAsync(connection, AddedColumnsForVersion(63))", block, StringComparison.Ordinal); Assert.Contains("ADD COLUMN IF NOT EXISTS {column} {type}", block, StringComparison.Ordinal); Assert.Contains("Running migration to v63", block, StringComparison.Ordinal); foreach (var phrase in new[] diff --git a/Lite/Database/DuckDbInitializer.cs b/Lite/Database/DuckDbInitializer.cs index 1669406731..36866539a2 100644 --- a/Lite/Database/DuckDbInitializer.cs +++ b/Lite/Database/DuckDbInitializer.cs @@ -816,6 +816,18 @@ Just create tables with the current schema and stamp the version. */ await ExecuteNonQueryAsync(connection, indexStatement); } + /* #4727: re-apply the columns versions 60 to 65 added on EVERY start of an existing file, after the + table and index statements. A step whose ALTER failed (a transient fault during the one start that + ran it) is stamped done all the same, and a file can also carry the stamp without the column; without + this the table's batches fail on every start from then on. The stamp is not held back: the pass + below heals a stamped file, and a column that still cannot be added logs an Error and lets the + start continue, so the next start retries it. A fresh file gets the columns from the table + statements above. */ + if (existingVersion > 0) + { + await AddMissingColumnsAsync(connection, AddedColumns); + } + if (existingVersion < CurrentSchemaVersion) { await SetSchemaVersionAsync(connection, CurrentSchemaVersion); @@ -2082,18 +2094,7 @@ interval is the ONLY thing that tells those apart. These four collectors discard Non-fatal per table, matching v59's posture. */ _logger?.LogInformation("Running migration to v60: the four naked delta families gain sample_interval_seconds"); - foreach (var table in new[] { "wait_stats", "file_io_stats", "latch_stats", "spinlock_stats" }) - { - try - { - await ExecuteNonQueryAsync(connection, - $"ALTER TABLE {table} ADD COLUMN IF NOT EXISTS sample_interval_seconds INTEGER"); - } - catch (Exception ex) - { - _logger?.LogWarning("Migration to v60 on {Table} encountered an error (non-fatal): {Error}", table, ex.Message); - } - } + await AddMissingColumnsAsync(connection, AddedColumnsForVersion(60)); } if (fromVersion < 61) @@ -2138,24 +2139,7 @@ key nothing will ever present. (CreateArchiveViewsAsync, called after this). Non-fatal per statement, matching v59/v60. */ _logger?.LogInformation("Running migration to v61: every delta family stores its interval, and query_stats stores the statement offsets its delta key is made of"); - foreach (var (table, column) in new[] - { - ("procedure_stats", "sample_interval_seconds"), - ("memory_grant_stats", "sample_interval_seconds"), - ("query_stats", "statement_start_offset"), - ("query_stats", "statement_end_offset"), - }) - { - try - { - await ExecuteNonQueryAsync(connection, - $"ALTER TABLE {table} ADD COLUMN IF NOT EXISTS {column} INTEGER"); - } - catch (Exception ex) - { - _logger?.LogWarning("Migration to v61 on {Table}.{Column} encountered an error (non-fatal): {Error}", table, column, ex.Message); - } - } + await AddMissingColumnsAsync(connection, AddedColumnsForVersion(61)); } if (fromVersion < 62) @@ -2189,15 +2173,7 @@ rebuilds every v_ view on start (CreateArchiveViewsAsync, called after this). No matching v59/v60/v61. */ _logger?.LogInformation("Running migration to v62: perfmon_stats stores each counter's type, so gauges are no longer differenced"); - try - { - await ExecuteNonQueryAsync(connection, - "ALTER TABLE perfmon_stats ADD COLUMN IF NOT EXISTS cntr_type INTEGER"); - } - catch (Exception ex) - { - _logger?.LogWarning("Migration to v62 on perfmon_stats.cntr_type encountered an error (non-fatal): {Error}", ex.Message); - } + await AddMissingColumnsAsync(connection, AddedColumnsForVersion(62)); } if (fromVersion < 63) @@ -2251,22 +2227,7 @@ parquet are unaffected (v_ views UNION ALL BY NAME the parquet with union_by_nam (CreateArchiveViewsAsync, called after this). Non-fatal per statement, matching v59–v62. */ _logger?.LogInformation("Running migration to v63: cpu_utilization_stats stores each sample's UTC instant beside the server-local one, and server_properties stores the engine's time-zone id beside its offset"); - foreach (var (table, column, type) in new[] - { - ("cpu_utilization_stats", "sample_time_utc", "TIMESTAMP"), - ("server_properties", "time_zone_id", "VARCHAR"), - }) - { - try - { - await ExecuteNonQueryAsync(connection, - $"ALTER TABLE {table} ADD COLUMN IF NOT EXISTS {column} {type}"); - } - catch (Exception ex) - { - _logger?.LogWarning("Migration to v63 on {Table}.{Column} encountered an error (non-fatal): {Error}", table, column, ex.Message); - } - } + await AddMissingColumnsAsync(connection, AddedColumnsForVersion(63)); } if (fromVersion < 64) @@ -2304,22 +2265,7 @@ recover a capture mode from the other nine options. Query Store clutter view (#3797) is the consumer. */ _logger?.LogInformation("Running migration to v64: query_store_health stores each database's Query Store capture modes, so plan churn can be told apart from configuration"); - foreach (var (table, column, type) in new[] - { - ("query_store_health", "query_capture_mode", "VARCHAR"), - ("query_store_health", "wait_stats_capture_mode", "VARCHAR"), - }) - { - try - { - await ExecuteNonQueryAsync(connection, - $"ALTER TABLE {table} ADD COLUMN IF NOT EXISTS {column} {type}"); - } - catch (Exception ex) - { - _logger?.LogWarning("Migration to v64 on {Table}.{Column} encountered an error (non-fatal): {Error}", table, column, ex.Message); - } - } + await AddMissingColumnsAsync(connection, AddedColumnsForVersion(64)); } if (fromVersion < 65) @@ -2348,21 +2294,96 @@ an existing database and are idempotent. Neither AG table has ever had a v_ pass per statement, matching v59–v64. */ _logger?.LogInformation("Running migration to v65: ag_replica_states and ag_database_replica_states store each Availability Group's engine-assigned id, closing a replica-name-overlap gap in the distinct-group count"); - foreach (var table in new[] + await AddMissingColumnsAsync(connection, AddedColumnsForVersion(65)); + } + } + + /// + /// #4727: the 15 columns that schema versions 60 to 65 add to existing tables, in ONE list. Each migration + /// step adds its own version's entries (), and every start of an existing + /// file runs the whole list once more after the table and index statements, so a column whose add failed + /// during the one start that ran its step, or a file stamped without it, gets it back on the next start. + /// A fresh file gets all of them from the table statements. A later version that adds a column to an + /// existing table adds its entries here. + /// + internal static readonly (int Version, string Table, string Column, string Type)[] AddedColumns = + { + (60, "wait_stats", "sample_interval_seconds", "INTEGER"), + (60, "file_io_stats", "sample_interval_seconds", "INTEGER"), + (60, "latch_stats", "sample_interval_seconds", "INTEGER"), + (60, "spinlock_stats", "sample_interval_seconds", "INTEGER"), + (61, "procedure_stats", "sample_interval_seconds", "INTEGER"), + (61, "memory_grant_stats", "sample_interval_seconds", "INTEGER"), + (61, "query_stats", "statement_start_offset", "INTEGER"), + (61, "query_stats", "statement_end_offset", "INTEGER"), + (62, "perfmon_stats", "cntr_type", "INTEGER"), + (63, "cpu_utilization_stats", "sample_time_utc", "TIMESTAMP"), + (63, "server_properties", "time_zone_id", "VARCHAR"), + (64, "query_store_health", "query_capture_mode", "VARCHAR"), + (64, "query_store_health", "wait_stats_capture_mode", "VARCHAR"), + (65, "ag_replica_states", "group_id", "VARCHAR"), + (65, "ag_database_replica_states", "group_id", "VARCHAR"), + }; + + internal static IEnumerable<(int Version, string Table, string Column, string Type)> AddedColumnsForVersion(int version) => + AddedColumns.Where(c => c.Version == version); + + /// + /// #4727: adds each listed column its table lacks, one idempotent ADD COLUMN IF NOT EXISTS per + /// missing column. The migration steps for versions 60 to 65 call it over their own entries, and + /// calls it over on every start of an existing + /// file. One read finds which tables and columns exist, so a start where nothing is missing runs no ALTER, + /// and a table that does not exist yet (a file older than the table: the table statements that follow + /// create it with the column) is skipped rather than reported as a failure. ADD COLUMN goes through on a + /// table that has an index or a v_ view over it (checked against the bundled DuckDB); only DROP COLUMN and + /// ALTER COLUMN hit the Dependency Error, so no index has to be dropped first. A column that still cannot be + /// added logs one Error naming the table and column and the loop moves on, so the start continues and the + /// next start retries it. + /// + internal async Task AddMissingColumnsAsync(DuckDBConnection connection, IEnumerable<(int Version, string Table, string Column, string Type)> columns) + { + var wanted = columns.ToList(); + if (wanted.Count == 0) + return; + + var tablesPresent = new HashSet(StringComparer.OrdinalIgnoreCase); + var columnsPresent = new HashSet(StringComparer.OrdinalIgnoreCase); + try + { + using var probe = connection.CreateCommand(); + probe.CommandText = "SELECT table_name, column_name FROM information_schema.columns WHERE table_schema = 'main' AND table_name IN (" + + string.Join(", ", wanted.Select(c => $"'{c.Table}'").Distinct()) + ")"; + using var reader = await probe.ExecuteReaderAsync(); + while (await reader.ReadAsync()) { - "ag_replica_states", - "ag_database_replica_states", - }) + tablesPresent.Add(reader.GetString(0)); + columnsPresent.Add(reader.GetString(0) + "." + reader.GetString(1)); + } + } + catch (Exception ex) + { + _logger?.LogError("Could not read which columns exist, so the check for missing schema columns was skipped; the next start retries it: {Error}", ex.Message); + return; + } + + foreach (var (version, table, column, type) in wanted) + { + if (!tablesPresent.Contains(table) || columnsPresent.Contains(table + "." + column)) + continue; + + try { - try - { - await ExecuteNonQueryAsync(connection, - $"ALTER TABLE {table} ADD COLUMN IF NOT EXISTS group_id VARCHAR"); - } - catch (Exception ex) - { - _logger?.LogWarning("Migration to v65 on {Table}.group_id encountered an error (non-fatal): {Error}", table, ex.Message); - } + /* Not ExecuteNonQueryAsync: that helper logs its own Error with the whole statement before it + rethrows, and a failed add must log exactly ONE Error, the one below that names the column. */ + using var alter = connection.CreateCommand(); + alter.CommandText = $"ALTER TABLE {table} ADD COLUMN IF NOT EXISTS {column} {type}"; + await alter.ExecuteNonQueryAsync(); + _logger?.LogInformation("Added missing column {Table}.{Column} {Type} (schema v{Version})", table, column, type, version); + } + catch (Exception ex) + { + _logger?.LogError("Adding column {Table}.{Column} {Type} (schema v{Version}) failed; the start continues and the next start retries it: {Error}", + table, column, type, version, ex.Message); } } } From d35667fdfa0220cac21be930702e8e64ef0bc96b Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 29 Sep 2026 05:13:47 -0400 Subject: [PATCH 3/4] Lite: after sleep and resume, each due collector runs once, not twice a minute apart (#4728) After a sleep the collection loop's delay returned long after the slot it waited for, and the loop handed the collectors that stale slot, so every collector due then ran, and ran again at the next slot a minute later. The wait now lives in an internal static method, WaitForNextCycleAsync, that takes the clock and the delay. When the delay returns a whole interval or more past its slot, the cycle starts at the latest grid slot at or before now (the same arithmetic as Darling's ServedSlot). A wait that returns on time is unchanged. The test drives that method with a fake clock and a fake delay and mark-and-due over 1, 5 and 15 minute collectors across a simulated 47 minute sleep: each due collector runs once after the resume, and the 1 minute collector still runs at the resume slot and the next one. --- Lite.Tests/CollectionCycleResumeTests.cs | 129 +++++++++++++++++++ Lite/Services/CollectionBackgroundService.cs | 40 ++++-- 2 files changed, 161 insertions(+), 8 deletions(-) create mode 100644 Lite.Tests/CollectionCycleResumeTests.cs diff --git a/Lite.Tests/CollectionCycleResumeTests.cs b/Lite.Tests/CollectionCycleResumeTests.cs new file mode 100644 index 0000000000..ba0fd2b2a5 --- /dev/null +++ b/Lite.Tests/CollectionCycleResumeTests.cs @@ -0,0 +1,129 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using PerformanceMonitorLite.Models; +using PerformanceMonitorLite.Services; +using Xunit; + +namespace PerformanceMonitorLite.Tests; + +/// +/// #4728: after the computer sleeps and resumes, the collection loop's delay returns long after the slot it waited +/// for. Handing the collectors that stale slot ran every collector that was due then, and ran it again at the next +/// slot a minute later. The wait returns the latest grid slot at or before now instead, so each due collector runs +/// once. The simulation drives the production wait () +/// with a fake clock and a fake delay, and 's mark-and-due for the collectors, the way +/// RemoteCollectorService marks a run with the cycle's slot. +/// +public sealed class CollectionCycleResumeTests : IDisposable +{ + private static readonly DateTime T0 = new(2026, 9, 29, 10, 0, 0, DateTimeKind.Utc); + private static readonly TimeSpan Interval = TimeSpan.FromMinutes(1); + private const string ServerId = "resume-server"; + private const string EveryMinute = "every_minute"; + private const string EveryFive = "every_five"; + private const string EveryFifteen = "every_fifteen"; + + private readonly string _configDir; + + public CollectionCycleResumeTests() + { + _configDir = Path.Combine(Path.GetTempPath(), "CollectionCycleResumeTests_" + Guid.NewGuid().ToString("N")[..8]); + Directory.CreateDirectory(_configDir); + } + + public void Dispose() + { + try { if (Directory.Exists(_configDir)) Directory.Delete(_configDir, recursive: true); } + catch { /* best-effort cleanup */ } + } + + private sealed record Run(string Collector, DateTime Slot, DateTime WallClock); + + /// + /// Runs loop passes from slot T0. The delay after pass + /// comes back late, the way a resume from sleep does (-1 for no sleep). + /// + private async Task> SimulateAsync(int iterations, int sleepAfterPass, TimeSpan sleep) + { + var schedule = new ScheduleManager(_configDir); + schedule.SetScheduleForServer(ServerId, new List + { + new() { Name = EveryMinute, Enabled = true, FrequencyMinutes = 1 }, + new() { Name = EveryFive, Enabled = true, FrequencyMinutes = 5 }, + new() { Name = EveryFifteen, Enabled = true, FrequencyMinutes = 15 }, + }); + + var now = T0; + var cycleStart = T0; + var runs = new List(); + for (var pass = 0; pass < iterations; pass++) + { + foreach (var due in schedule.GetDueCollectorsForServer(ServerId, cycleStart)) + { + runs.Add(new Run(due.Name, cycleStart, now)); + schedule.MarkCollectorRunForServer(ServerId, due.Name, cycleStart); + } + + var thisPass = pass; + cycleStart = await CollectionBackgroundService.WaitForNextCycleAsync( + cycleStart, + Interval, + () => now, + (wait, _) => + { + now += wait; + if (thisPass == sleepAfterPass) + now += sleep; + return Task.CompletedTask; + }, + CancellationToken.None); + } + + return runs; + } + + /// + /// A 47-minute sleep begins while the loop waits for the slot at which the 5-minute collector becomes due. Each + /// due collector runs once after the resume: the 5-minute one is not run again a minute later (the old shape ran + /// it at the stale slot and again at the next one, 60 s apart), and the resume slot is on the grid. + /// + [Fact] + public async Task AfterASleepAndResume_EachDueCollectorRunsOnce_NotTwiceAMinuteApart() + { + var sleep = TimeSpan.FromMinutes(47); + var runs = await SimulateAsync(iterations: 12, sleepAfterPass: 4, sleep); + + var resumeWall = T0 + TimeSpan.FromMinutes(5) + sleep; + var afterResume = runs.Where(r => r.WallClock >= resumeWall).ToList(); + + /* The slot handed to the collectors is the latest grid slot at or before now, not the one from before the sleep. */ + Assert.All(afterResume, r => Assert.Equal(r.WallClock, r.Slot)); + + var five = afterResume.Where(r => r.Collector == EveryFive).ToList(); + Assert.Equal(new[] { resumeWall, resumeWall + TimeSpan.FromMinutes(5) }, five.Select(r => r.WallClock).ToArray()); + + var fifteen = afterResume.Where(r => r.Collector == EveryFifteen).ToList(); + Assert.Equal(new[] { resumeWall }, fifteen.Select(r => r.WallClock).ToArray()); + + /* A 1-minute collector still runs at the resume slot and at the next one. */ + var minute = afterResume.Where(r => r.Collector == EveryMinute).Select(r => r.Slot).ToList(); + Assert.Contains(resumeWall, minute); + Assert.Contains(resumeWall + Interval, minute); + } + + /// Without a sleep the grid is unchanged: each collector runs at its own cadence, at exact slots. + [Fact] + public async Task WithoutASleep_EachCollectorRunsAtItsOwnCadenceOnTheGrid() + { + var runs = await SimulateAsync(iterations: 16, sleepAfterPass: -1, TimeSpan.Zero); + + Assert.Equal(16, runs.Count(r => r.Collector == EveryMinute)); + Assert.Equal(new[] { 0, 5, 10, 15 }, runs.Where(r => r.Collector == EveryFive).Select(r => (int)(r.Slot - T0).TotalMinutes).ToArray()); + Assert.Equal(new[] { 0, 15 }, runs.Where(r => r.Collector == EveryFifteen).Select(r => (int)(r.Slot - T0).TotalMinutes).ToArray()); + Assert.All(runs, r => Assert.Equal(r.WallClock, r.Slot)); + } +} diff --git a/Lite/Services/CollectionBackgroundService.cs b/Lite/Services/CollectionBackgroundService.cs index 4a27a6c08b..e56f7dfd7b 100644 --- a/Lite/Services/CollectionBackgroundService.cs +++ b/Lite/Services/CollectionBackgroundService.cs @@ -245,14 +245,8 @@ property reads — negligible overhead at 1-minute cadence. */ try { - var nextCycle = CollectorCadence.NextDue(cycleStart, DateTime.UtcNow, CollectionInterval); - var wait = nextCycle - DateTime.UtcNow; - if (wait > TimeSpan.Zero) - { - await Task.Delay(wait, stoppingToken); - } - - cycleStart = nextCycle; + cycleStart = await WaitForNextCycleAsync(cycleStart, CollectionInterval, () => DateTime.UtcNow, + (wait, token) => Task.Delay(wait, token), stoppingToken); } catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested) { @@ -263,6 +257,36 @@ property reads — negligible overhead at 1-minute cadence. */ _logger?.LogInformation("Collection background service stopped"); } + /// + /// #4728: waits for the next slot on the collector grid and returns the logical start of the cycle to run. + /// The wait is normally one interval, and becomes the slot it waited for. After + /// the computer sleeps and resumes, though, the delay returns long after that slot, and handing the collectors + /// a slot from before the sleep would run every collector that was due then and run it again at the next slot + /// a minute later. When the delay returns a whole interval or more past its slot, the cycle starts at the + /// latest grid slot at or before now instead (the same arithmetic as Darling's ServedSlot): a slot missed + /// during a stall is skipped, not replayed (). Static, with the clock + /// and the delay passed in, so a test drives this path with a fake clock and a fake delay. + /// + internal static async Task WaitForNextCycleAsync( + DateTime cycleStart, + TimeSpan interval, + Func utcNow, + Func delay, + CancellationToken cancellationToken) + { + var nextCycle = CollectorCadence.NextDue(cycleStart, utcNow(), interval); + var wait = nextCycle - utcNow(); + if (wait > TimeSpan.Zero) + { + await delay(wait, cancellationToken); + } + + var now = utcNow(); + return now >= nextCycle + interval + ? CollectorCadence.NextDue(nextCycle, now, interval) - interval + : nextCycle; + } + /// #2058: fills the Query Store history the live path never takes — the 60-minute /// first-contact tail and clamp-bounded outage holes — newest-first, strictly behind the live /// path's floor, never past the resolved query_store retention. See From ca986721510a367bb2183c316a7ca3e2beeed901 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Tue, 29 Sep 2026 05:43:26 -0400 Subject: [PATCH 4/4] Lite schema pins: the v63 and v64 tests read the versioned AddedColumns entries and the per-version step calls (#4727) Two Darling tests read Lite's DuckDbInitializer as text and still looked for the old three-field column tuples, which #4727 replaced with the four-field entries of DuckDbInitializer.AddedColumns (version first). Each test now pins its columns as entries of that list, and pins that its own migration step calls AddMissingColumnsAsync(connection, AddedColumnsForVersion(N)) for its own version. The step is cut at the next "if (fromVersion <", so a call in a later step cannot satisfy the pin. The schema-version floor, the "twinning Darling's V134" / "V137" anchors, the "Running migration to v64" log line and the golden-file checks are unchanged. --- .../QsCaptureModeRouteKnobToastRungTests.cs | 17 +++++++++++++--- Darling/Darling.Tests/TimeHonestyRungTests.cs | 20 ++++++++++++++++--- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/Darling/Darling.Tests/QsCaptureModeRouteKnobToastRungTests.cs b/Darling/Darling.Tests/QsCaptureModeRouteKnobToastRungTests.cs index 47b949343b..abe0e65673 100644 --- a/Darling/Darling.Tests/QsCaptureModeRouteKnobToastRungTests.cs +++ b/Darling/Darling.Tests/QsCaptureModeRouteKnobToastRungTests.cs @@ -391,9 +391,20 @@ being the ceiling. */ Assert.True(int.Parse(versionMatch.Groups[1].Value) >= 64); var start = initializer.IndexOf("if (fromVersion < 64)", StringComparison.Ordinal); Assert.True(start >= 0, "DuckDbInitializer has no v64 block"); - var block = initializer[start..]; - Assert.Contains("(\"query_store_health\", \"query_capture_mode\", \"VARCHAR\")", block, StringComparison.Ordinal); - Assert.Contains("(\"query_store_health\", \"wait_stats_capture_mode\", \"VARCHAR\")", block, StringComparison.Ordinal); + /* #4727: the two columns are entries of ONE versioned list, DuckDbInitializer.AddedColumns (version first), + and the v64 step adds its own version's entries through the shared AddMissingColumnsAsync. The step is cut + at the next "if (fromVersion <" so a call in a later step cannot satisfy the pin. */ + var end = initializer.IndexOf("if (fromVersion <", start + 1, StringComparison.Ordinal); + Assert.True(end > start, "DuckDbInitializer's v64 block has no later step to end at"); + var block = initializer[start..end]; + var listStart = initializer.IndexOf("internal static readonly (int Version, string Table, string Column, string Type)[] AddedColumns =", StringComparison.Ordinal); + Assert.True(listStart >= 0, "DuckDbInitializer no longer declares AddedColumns in the pinned shape"); + var listEnd = initializer.IndexOf("};", listStart, StringComparison.Ordinal); + Assert.True(listEnd > listStart, "DuckDbInitializer's AddedColumns list has no closing brace to end at"); + var list = initializer[listStart..listEnd]; + Assert.Contains("(64, \"query_store_health\", \"query_capture_mode\", \"VARCHAR\")", list, StringComparison.Ordinal); + Assert.Contains("(64, \"query_store_health\", \"wait_stats_capture_mode\", \"VARCHAR\")", list, StringComparison.Ordinal); + Assert.Contains("AddMissingColumnsAsync(connection, AddedColumnsForVersion(64))", block, StringComparison.Ordinal); Assert.Contains("Running migration to v64", block, StringComparison.Ordinal); Assert.Contains("twinning Darling's V137", block, StringComparison.Ordinal); diff --git a/Darling/Darling.Tests/TimeHonestyRungTests.cs b/Darling/Darling.Tests/TimeHonestyRungTests.cs index 708255164a..95b344fdfc 100644 --- a/Darling/Darling.Tests/TimeHonestyRungTests.cs +++ b/Darling/Darling.Tests/TimeHonestyRungTests.cs @@ -205,9 +205,23 @@ public void TheLiteTwinIsSchemaV63_AndCarriesBothColumns() var declaration = System.Text.RegularExpressions.Regex.Match(lite, @"internal const int CurrentSchemaVersion = (\d+);"); Assert.True(declaration.Success, "DuckDbInitializer no longer declares CurrentSchemaVersion in the pinned shape"); Assert.True(int.Parse(declaration.Groups[1].Value, CultureInfo.InvariantCulture) >= 63, "Lite's schema version fell below the v63 twin"); - var block = lite[lite.IndexOf("if (fromVersion < 63)", StringComparison.Ordinal)..]; - Assert.Contains("(\"cpu_utilization_stats\", \"sample_time_utc\", \"TIMESTAMP\")", block, StringComparison.Ordinal); - Assert.Contains("(\"server_properties\", \"time_zone_id\", \"VARCHAR\")", block, StringComparison.Ordinal); + /* #4727: Lite's added columns are entries of ONE versioned list, DuckDbInitializer.AddedColumns (version + first), and each migration step adds its own version's entries through the shared AddMissingColumnsAsync. + So the two v63 columns are pinned as entries of that list, and the v63 step, cut at the next + "if (fromVersion <" so a call in a later step cannot satisfy the pin, as the one that adds version 63's. */ + var listStart = lite.IndexOf("internal static readonly (int Version, string Table, string Column, string Type)[] AddedColumns =", StringComparison.Ordinal); + Assert.True(listStart >= 0, "DuckDbInitializer no longer declares AddedColumns in the pinned shape"); + var listEnd = lite.IndexOf("};", listStart, StringComparison.Ordinal); + Assert.True(listEnd > listStart, "DuckDbInitializer's AddedColumns list has no closing brace to end at"); + var list = lite[listStart..listEnd]; + Assert.Contains("(63, \"cpu_utilization_stats\", \"sample_time_utc\", \"TIMESTAMP\")", list, StringComparison.Ordinal); + Assert.Contains("(63, \"server_properties\", \"time_zone_id\", \"VARCHAR\")", list, StringComparison.Ordinal); + var start = lite.IndexOf("if (fromVersion < 63)", StringComparison.Ordinal); + Assert.True(start >= 0, "DuckDbInitializer has no v63 block"); + var end = lite.IndexOf("if (fromVersion <", start + 1, StringComparison.Ordinal); + Assert.True(end > start, "DuckDbInitializer's v63 block has no later step to end at"); + var block = lite[start..end]; + Assert.Contains("AddMissingColumnsAsync(connection, AddedColumnsForVersion(63))", block, StringComparison.Ordinal); Assert.Contains("twinning Darling's V134", block, StringComparison.Ordinal); }