Fix two aged-database migration bugs behind #2748's Lite startup failure

[erikdarlingdata]

Part of #2748.

What's actually wrong (two distinct bugs, both empirically confirmed against real DuckDB 1.5.5)

v48 (server_properties NOT NULL relaxation) fails on any database that has completed a prior startup. DuckDbSchemaGenerator's default-case index (idx_server_properties_time) already exists by then, and DuckDB's ALTER COLUMN dependency check refuses a table with any index on it — even one naming none of the altered columns. I initially guessed this was the archive view (v_server_properties) and was wrong; a standalone repro against the pinned DuckDB.NET 1.5.5 showed a plain SELECT * view does not trigger the dependency error, only the index does. Fix: drop idx_server_properties_time before the three ALTER COLUMN statements; the schema loop's unconditional index-creation step (already runs right after migrations, every startup) recreates it.

v53 (config_database_state_expected new columns) fails on any database old enough to predate the table's introduction (#2166/#2203). That table has no numbered CREATE migration of its own — it only exists via Schema.GetAllTableStatements()'s unconditional CREATE TABLE IF NOT EXISTS, which doesn't run until after RunMigrationsAsync returns. v54's migration already does this correctly (create-then-alter, idempotent); v53 never did. Fix: call the same idempotent CreateDatabaseStateExpectedTable DDL before the two ALTER statements, matching v54's established pattern.

Both reproduce the exact logged error text from the issue, character for character (confirmed via a standalone DuckDB.NET 1.5.5 script, not just by reading the code).

What this PR does not confirm

Neither bug is confirmed as the actual "app does not start" symptom:

• v53's failure self-heals moments later regardless of the fix — the unconditional CREATE TABLE IF NOT EXISTS runs right after migrations either way, so the table ends up correctly shaped even without this fix, for this specific user's case (table didn't exist at all).
• v48's collector writes happen on a fire-and-forget background task (_ = Task.Run(() => _backgroundService.StartAsync(...)) in MainWindow_Loaded) — a NOT NULL violation there would not surface as a startup-blocking exception visible to the user.

Both are real, independently worth fixing, and match the two errors the user's log actually shows — but the pasted log cuts off right after migrations complete ("Analysis schema initialized at version 5"), before whatever actually blocked startup. I'm commenting on the issue to ask for the rest of the log rather than claiming this closes it.

Testing

• Standalone DuckDB.NET 1.5.5 repro script reproducing both exact errors pre-fix and confirming success post-fix (not committed, just used to validate before touching the real migration code).
• Lite.Tests/AgedDatabaseMigrationTests.cs: seeds both real preconditions (an aged NOT NULL server_properties + its real default index; a v47 database missing config_database_state_expected entirely) against the actual DuckDbInitializer.InitializeAsync path, and asserts the upgrade both completes AND leaves the database in a genuinely correct, writable state — not just that it didn't throw.
• Could not run the test host locally (Lite.Tests needs the Windows Desktop runtime, unavailable on this Mac) — relying on CI.

[claude]

Reviewed. Note: this PR's diff against its recorded base (4bb851a, an older point on main) pulls in ~50 already-merged commits (the dev branch catch-up spanning #2671–#2690), so the diff is huge but not new. The actual unreviewed work is the tip commit fixing #2748: Lite/Database/DuckDbInitializer.cs (v48/v53 migration fixes) + the new Lite.Tests/AgedDatabaseMigrationTests.cs. Focused this review there.

Correctness — looks solid.

• v48 fix: confirmed DuckDbSchemaGenerator.CreateIndex's default case really does create idx_server_properties_time ON server_properties(server_id, collection_time) (DuckDbSchemaGenerator.cs:166), exactly matching the dropped/recreated index name and shape. Schema.GetAllIndexStatements() runs unconditionally right after migrations in InitializeAsync (DuckDbInitializer.cs:378-381), so the drop-then-recreate sequencing is correct and nothing is left dangling.
• v53 fix: confirmed Schema.CreateDatabaseStateExpectedTable (Schema.cs:184-197) already includes last_alerted_state/last_alerted_at, so calling it before the ALTERs is idempotent for existing tables and a correct fresh create for databases that predate the table — matches v54's established create-then-alter pattern in the same file.
• New tests seed the exact real preconditions (the real index DDL, a v47 database missing the table entirely) against the actual DuckDbInitializer.InitializeAsync path and assert post-upgrade writability, not just "didn't throw." Good regression coverage.
• No SQL injection concerns — both new statements are static literals, no interpolation.
• No Lite/Darling parity issue: Darling's Postgres equivalent (PgMigrations.cs V60) was already created via its own numbered migration, so it never had the "table doesn't exist until after migrations run" problem this fix addresses — that architecture gap is Lite/DuckDB-specific.

Minor, non-blocking: the catch { /* Table doesn't exist yet — will be created with the full schema below */ } at DuckDbInitializer.cs:1417-1420 is now stale — the table is unconditionally created a few lines above inside the same try, so that comment's premise no longer holds if this catch ever fires. Also worth noting this is the only bare (non-logging) catch among the migration blocks in this file (every sibling logs via _logger?.LogWarning) — given the whole PR is about swallowed-migration-failure bugs, adding a log line here would make a future silent failure of this kind actually visible instead of repeating the pattern that caused #2748.

[claude]

Good fix — matches v54's create-then-alter pattern and Schema.CreateDatabaseStateExpectedTable already has both new columns, so the two ALTERs below are no-ops on a fresh create.

One thing worth tightening: the catch { } a few lines below (line ~1419, "Table doesn't exist yet — will be created with the full schema below") is now stale, since the table is unconditionally created right here inside the same try. If that catch still fires, it's for some other reason, not a missing table. It's also the only bare, non-logging catch among the migration blocks in this file — every sibling (v47, v48, v49, v54) logs via _logger?.LogWarning. Given this whole PR exists to fix a silently-swallowed migration failure (#2748), adding a log line here would surface a future failure of the same shape instead of repeating it.

[erikdarlingdata]

Fixed in 0ebf8e8 — now logs via _logger?.LogWarning, matching the sibling migration blocks.

[claude]

Reviewed. Both fixes are correct and well-verified:

• v48 index-drop fix: idx_server_properties_time is indeed the only index DuckDB's schema generator creates for server_properties (the default case in DuckDbSchemaGenerator.CreateIndex, server_id/collection_time), so dropping it before the three ALTER COLUMN ... DROP NOT NULL statements is the right fix, and GetAllIndexStatements() unconditionally recreates it right after RunMigrationsAsync returns (DuckDbInitializer.cs:378) within the same InitializeAsync call, so nothing is left dangling even on a fresh run.
• v53 create-before-alter fix: Schema.CreateDatabaseStateExpectedTable is a plain CREATE TABLE IF NOT EXISTS that already includes last_alerted_state/last_alerted_at (Schema.cs:184-197), so all three cases (table absent / table present without the columns / table present with the columns) resolve correctly through create-then-alter — this now mirrors v54's established pattern exactly, as claimed.
• Swallowing-to-logging change on the v53 catch is a real improvement and consistent with every sibling migration block's style (LogWarning with the caught exception's message).
• No SQL injection or security concerns — all DDL is static/constant, nothing derived from user input.
• No Lite/Darling parity gap here: Darling's config.database_state_expected is created by its own numbered rung (V49) strictly before the V60/V61 ALTERs in the same migration ladder, so it can't hit the "ALTER before CREATE" class of bug Lite just had (Lite's bug is specific to table creation living outside the numbered chain, in the unconditional post-migration GetAllTableStatements() loop). The "any index blocks ALTER COLUMN" restriction is DuckDB-specific and doesn't apply to Darling's Postgres migrations either.
• New AgedDatabaseMigrationTests.cs exercises both bugs against the real DuckDbInitializer.InitializeAsync path with the exact seeded preconditions (aged NOT NULL columns + real default index; a v47 DB missing the table entirely) and asserts post-upgrade writability, not just "didn't throw" — good regression coverage for a bug class that's easy to silently reintroduce.

One minor (non-blocking) nit: the seeded server_properties table in the first test omits the server_name VARCHAR NOT NULL / id prefix columns that DuckDbSchemaGenerator.CreateTable always emits for the real table (DuckDbSchemaGenerator.cs:94-103). Since the table already exists after the seed, GetAllTableStatements()'s CREATE TABLE IF NOT EXISTS won't backfill those columns, so the test's final table shape diverges slightly from production. Doesn't affect what the test is actually verifying (the index/ALTER interaction), just flagging for awareness.

No blocking issues found.