Repository navigation
Add V215: six indexes on the agent investigation tables - #6637
Conversation
A one-year synthetic load on a restored store copy showed follow-up, request list, brief and retention reads slowing as requests build up. Six partial and plain indexes bring each back under 10 ms.
There was a problem hiding this comment.
Verdict: CHANGES REQUESTED
The rung is clean. One thing needs fixing, and it is a sentence in the body, not a line of code.
Claims
- VERIFIED - V215
agent-table-indexesadds the six named indexes - PgMigrations.cs:2305-2317, registered :2708. - VERIFIED - SchemaVersion is 215 - StorageVersion.cs:19, pinned symbolically at AgentLevel2StoreTests.cs:50.
- VERIFIED - Probe sentinel plus newest-first arm, all four parts - ViewerDataService.cs:1337, :1587-1590, :1381, :1359. 191 top-level probe expressions (765-1337) against 191
bool hasparameters, so GetBoolean(190) cannot run off the end. V214 correctly demoted at :1591. - VERIFIED - Ladder entry and drop SQL present - RungRewindArtifacts.cs:508-512; DROP INDEX IF EXISTS applies per name, safe below V184 where the table is absent.
- VERIFIED - No test pinned 214 as the top; the one literal 214 is V214's own feature gate, which already has a 215 case - ViewerKnownPatternEditorTests.cs:100-101.
- VERIFIED - V184's InvestigationTables.CreateSql untouched - not among the five changed files.
- VERIFIED - A fresh store runs every rung - PgMigrations.cs:9595-9600, the path the new test takes via OpenStoreAsync.
- VERIFIED - No provisioning change needed; the six names appear nowhere else, and the README rollback list is explicitly non-exhaustive - Darling/README.md:295.
- VERIFIED - Predicates 1 and 2 - ProcedureStatsModuleDates.cs:44,46,48 and InvestigationRequestStore.cs:185,229 (a strict equality implies NOT NULL).
- VERIFIED - Predicates 3 and 4 - InvestigationRequestStore.cs:225,227,230 and :419, :485-486.
- VERIFIED - Predicates 5 and 6 - InvestigationRequestStore.cs:523 and PgInvestigationBriefSourceReader.cs:109,150,251; family_of is the only one of five FKs into the requests table whose column lacked a leading-column index, the other four leading on request_id (InvestigationTables.cs:134,156, PgMigrations.cs:1506-1507,1617). That is what makes the sixth index the right fix for the 310 ms.
- VERIFIED - The new test follows the established pattern and its seed satisfies every NOT NULL column and both CHECKs - AgentTableIndexesV215LiveTests.cs:32-78 against PgStatementTextDueIndexLiveTests.cs:28-60 and InvestigationTables.cs:78-113.
- UNVERIFIED - The before/after wall-millisecond table - needs the restored store copy and the synthetic year.
- UNVERIFIED - "Each index was tested inside a rolled-back transaction" - a property of the session, not the tree.
- UNVERIFIED - "Remove index 1 and the first assert fails" - needs a live DARLING_TEST_PG and a mutation run.
- UNVERIFIED - The local twice-applied scratch container and its plan result - needs the container.
- UNVERIFIED - "#6619 and #6635 also say V215" - another PR; only 215's density here is checkable (MigrationLadderPins.cs:135-148).
- UNVERIFIED - "The resolve check is fixed separately in #6633" - another PR.
- REFUTED - "the investigation tables are a Darling feature". Lite defines investigation_requests and investigation_reports (Lite/Database/Schema.cs:429,491), registers both in GetAllTableStatements() (:591,593), ships Lite/Mcp/McpInvestigationTools.cs and Lite/Services/InvestigationSweepService.cs, and the changelog records "Lite: Level 1 agent investigations" (CHANGELOG.md:45). Lite carries the counterpart reads too: the same follow_up_of anti-join at DuckDbInvestigationStore.cs:153 and opened_at filters at DuckDbInvestigationStore.Evidence.cs:69,81.
Riskiest lines
- ViewerDataService.cs:1337 - a wrong sentinel name, position or ordinal makes a current store probe 214 and the gate at MainWindow.xaml.cs:409 refuse it permanently - covered by AgPrimaryReplicaRungTests.AFreshStore_AndOneUpgradedFromThePriorRung_StoreAndReadBackThePrimary_WithTheColumnLast, which runs the real probe SQL on a migrated store and asserts SchemaVersion (:54,189-205); its :63-66 half also fails if the rewind entry misses the sentinel.
- PgMigrations.cs:2307 - a predicate the read's WHERE does not imply builds an index the planner can never use, costing writes for nothing with every assertion green - covered by AgentTableIndexesV215LiveTests.TheFollowUpModuleRead_UsesTheConditionIndex_AndNotOnceItIsDropped.
- PgMigrations.cs:2316 - the reports index. Its existence is covered via the probe, but nothing exercises the fk_investigation_reports_family ON DELETE SET NULL check using it, so a predicate the RI query could not use leaves the 310 ms in place, green - UNCOVERED.
Traps near this diff
- "Lite and Darling ship in parity (CONTRIBUTING's two-store rule): a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit" - does not apply - no reader, tool, alert or description here, and an index adds no state; the clause is scoped to "State added to one store" (CONTRIBUTING.md:309-313), and Lite's table has neither practice_of nor follow_up_condition and declares no index (Lite/Database/Schema.cs:429-465).
- "Ruling (#5833): tests are load-bearing. ... never answer a trap with a test that reads source text or asserts how code is spelled" - does not apply - the new test asserts a real plan and adds no census (AgentTableIndexesV215LiveTests.cs:50-78).
- "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." - does not apply - the names are DDL once (PgMigrations.cs:2306-2317) and catalog literals twice, the established sentinel shape (ViewerDataService.cs:770,793); a stale literal fails the live probe test rather than hiding behind a pin.
- "A test that reads the clock twice measures the CI scheduler ... One anchor owns both ends of an interval." (#3676) - does not apply - no duration is asserted and every seeded timestamp comes from one now() in a single statement (AgentTableIndexesV215LiveTests.cs:60-69).
- ".gitattributes says text=auto eol=crlf ... A diff that touches every line of a file is a line-ending conversion until proven otherwise." - does not apply - 127 additions, 5 deletions, five files; the long ViewerDataService.cs lines are genuinely one-call-per-line.
Divergence (blind read vs PR body)
The Lite rationale. "The investigation tables are a Darling feature" is contradicted by Lite's own schema, store, sweep service and MCP tools, and Lite carries the very reads these indexes serve. The conclusion - no Lite counterpart - is still right, for a different reason: DuckDB has no partial indexes, its ART index serves equality and uniqueness rather than the range scans and ORDER BY opened_at DESC these six are for, Lite declares no index on any investigation table, and a single-seat store never reaches the year of requests that made these reads slow. Please state that instead: "it's a Darling feature" would equally wave away a Lite twin for a reader, which the two-store rule calls a finding.
Smaller: the body credits MigrationProbeLadderTests with covering the probe. It covers the mapping only, by reflection over synthetic booleans (MigrationProbeLadderTests.cs:40-48), and never executes StoreSchemaProbeSql - a missing sentinel line or wrong GetBoolean index passes it. The live AgPrimaryReplicaRungTests path is what catches that, so a green unit run is not the probe being proven.
Reviewer checklist
- Run with DARLING_TEST_PG set: AgentTableIndexesV215LiveTests and AgPrimaryReplicaRungTests, the only test proving a V215 store probes 215 rather than 214.
- On real data, confirm the fk_investigation_reports_family check plans through ix_investigation_reports_family_of; it is a generic SPI plan, so verify in the field (riskiest line 3, uncovered).
- Confirm the builds take no noticeable lock on the largest store: CREATE INDEX takes ShareLock and the rung carries no SET LOCAL lock_timeout (matching V153, but resting on the unverifiable "hundreds of rows").
- Settle the V215 collision with #6619 and #6635, and confirm the loser renumbers rung and StorageVersion together.
|
Follow-up review (range This range contains exactly one new commit after the previously-reviewed Checked whether that merge is purely mechanical or carries any author-authored conflict resolution:
So this merge brings in the base branch's work and nothing else — no new author change, and no interaction with the already-reviewed V215 commit. Per the follow-up scope, that work isn't re-reviewed here, and the prior review's open finding (the "investigation tables are a Darling feature" rationale in the PR body) is unaffected by this commit, so it isn't repeated in this pass. Nothing in this range requires changes. |
There was a problem hiding this comment.
Verdict: LGTM
Claims
No new claims to grade: the only new commit in this range is cc3027d8f, a merge of origin/dev with an empty commit message body. The PR body's claims describe the earlier 7e94df73c commit, already graded in the prior full-pass review, so they are not re-graded here.
Riskiest lines
No lines changed by this range's author content. The merge is confirmed mechanical: git diff 7e94df73c cc3027d8f on the four files V215 touches (PgMigrations.cs, StorageVersion.cs, RungRewindArtifacts.cs, ViewerDataService.cs) is empty, and the six dev commits brought in by the merge don't touch those files at all (verified via their common merge-base ada395ecd).
Traps near this diff
None — there is no new authored diff in this range to hold against the traps file.
Divergence (blind read vs PR body)
None — the merge commit carries no message body and makes no claims to compare against the diff.
Reviewer checklist
- The prior review's open item (PR body's "the investigation tables are a Darling feature" rationale, refuted by Lite's own schema/store/MCP tools) is still unaddressed in the current PR body text — worth fixing before merge, but it predates this range and isn't a defect introduced by
cc3027d8f.
…ewind V215 in the below-V214 viewer test
Claude posting for Erik Darling. Answered at head 4064092. The Lite rationale and the probe-test claim were wrong in the body and are fixed. The FK-index test and lock_timeout points have no concrete trigger and are listed under Considered, not added. No code change was needed.
| /// (<c>InvestigationTables.CreateSql</c>) is not edited, and a fresh store runs every rung, so it gets these too. Additive: an older build | ||
| /// never reads the indexes, and no GRANT is needed for an index.</para> | ||
| /// </summary> | ||
| private const string V215Sql = @" |
There was a problem hiding this comment.
The body's lock_timeout precedent is inverted. "Considered, not added" says: "No lock_timeout on V215. It matches V153." V153 does set one — PgMigrations.cs:206:
private static readonly string V153Sql = @"
SET LOCAL lock_timeout = '" + (MigrationCommandTimeoutSeconds - 20) + @"s';
SET LOCAL max_parallel_maintenance_workers = 2;
CREATE INDEX IF NOT EXISTS idx_query_store_interval_latest_first_exec
ON collect.query_store_interval_latest (first_execution_time);";
So does V154 (:219), V152 (:100), V173 (:932) and V175 (:1008) — and :83-97 records why: those rungs take a lock a background refresh policy can hold for ~264 s, so the rung needs the server's own retryable 55P03 to fire before the client-side command timeout cancels the statement.
Omitting it here is the right call, but the precedent is the other set: the index-only rungs on non-hypertable tables set none — V104 (collect.pg_deadlocks), V113 (collect.plan_force_actions) and V207 (config.investigation_requests itself, ChangeWatchStore.cs:177). V207 is the exact shape: a partial index on this very table, no lock_timeout. Citing V153 instead tells the next reader that a rung racing a 264-second refresh doesn't need one, which is the failure :83-97 was written to prevent.
One-word fix in the body — V207, not V153. No code change.
|
|
||
| await ExecAsync(connection, "DROP INDEX config." + ConditionIndex, ct); | ||
| var without = await ExplainAsync(connection, ct); | ||
| Assert.DoesNotContain(ConditionIndex, without, StringComparison.Ordinal); |
There was a problem hiding this comment.
The drop half proves the harness, not the plan. Once DROP INDEX has run, no plan PostgreSQL can produce is able to name a dropped index, so this assertion fails on no defect in V215 — only on an ExplainAsync that returned a stale or constant string. That is worth something (it shows the EXPLAIN really is re-planned against the catalog), but it is not the control the class doc sells it as: "the read must go through V215's partial index on the condition, and must not once that index is gone."
The assertion that would fail on a defect is the one you already measured by hand — the body says "with the index it plans an Index Scan on it, and without it a Seq Scan." Assert the fallback:
| Assert.DoesNotContain(ConditionIndex, without, StringComparison.Ordinal); | |
| Assert.DoesNotContain(ConditionIndex, without, StringComparison.Ordinal); | |
| /* The control that fails on something: with the index gone the only path left is the whole table, so a plan that | |
| still finds a cheap one means the seed never made the index load-bearing and the first assertion was free. */ | |
| Assert.Contains("Seq Scan", without, StringComparison.Ordinal); |
That pins what the 127 ms → 5.5 ms measurement actually rests on: at a year of requests the read has no alternative to a full scan. Without it, a seed that quietly stops populating follow_up_condition (one row in 200, line 62) leaves both assertions green while the first has stopped discriminating.
Non-blocking.
The Lite rationale lands on the right answer by the wrong road"Considered, not added" says: "Lite has the same tables and reads, but DuckDB has no partial indexes, and its ART index serves equality rather than the range scans and
The conclusion (no Lite change) is right, and CONTRIBUTING's parity rule does not reach this rung either: Non-blocking. Things that held up, for the record
|
There was a problem hiding this comment.
Verdict: CHANGES REQUESTED
Claims
- VERIFIED — six indexes as rung V215 — PgMigrations.cs:2305-2317, Migration(215, "agent-table-indexes") at :2708
- VERIFIED — SchemaVersion is 215 — StorageVersion.cs:20
- VERIFIED — probe sentinel, newest-first arm returning 215, ladder entry with drop SQL — ViewerDataService.cs:1337, :1359, :1384, :1584-1588; RungRewindArtifacts.cs:508-515. 191 top-level probe expressions against 191 parameters, so the halves are in step
- VERIFIED — MigrationProbeLadderTests covers the mapping only — :46-47 builds the arguments from Enumerable.Range and opens no connection
- VERIFIED — the live AgPrimaryReplicaRungTests path runs the real probe SQL and proves a V215 store probes 215 — :194 executes StoreSchemaProbeSql, :54 and :72 assert StorageVersion.SchemaVersion
- VERIFIED — no literal 214 pins existed — the only 214s are V214's own ordinals (RungRewindArtifacts.cs:503, ViewerKnownPatternEditorTests.cs:100), which CONTRIBUTING.md:270-273 says never move
- VERIFIED — V184's InvestigationTables.CreateSql is not edited — absent from the changed files; :115-119 and :170-172 unchanged
- VERIFIED — provisioning needs no change — provision-roles.sql:2824-2866 grants per table and per column; a PostgreSQL index carries no ACL
- VERIFIED — predicate checks 1, 2, 3, 5, 6 — ProcedureStatsModuleDates.cs:44-46,48; InvestigationRequestStore.cs:185,225-229,523; InvestigationPracticeStore.cs:171,193; the ON DELETE SET NULL key at PgMigrations.cs:1591-1592
- VERIFIED — predicate check 4, ReportsRequestsSql and ListSql filter and order on opened_at — InvestigationRequestStore.cs:485-486, :419, :445
- VERIFIED — the plan-test pattern exists — Darling.Tests/PgStatementTextDueIndexLiveTests.cs
- VERIFIED — V215 declared SetsTheFloor: false, multiple stays 5 — MigrationDataMovingRungCensusPins.cs:332-341; the pin at :396-426 counts floor rungs only, and :352 dedupes per rung, so six statements need one line
- VERIFIED — no other test drops a V214 sentinel and then probes — PgInvestigationBriefSourceReaderLiveTests.cs:300-304 asserts on the reader, never on a probed version
- VERIFIED — Lite declares no index on any investigation table — Lite/Database/Schema.cs:416-417
- VERIFIED — index 6 leads on family_of so an equality probe can use it — the SET NULL action at PgMigrations.cs:1591-1592 updates on an equality, which implies NOT NULL and admits the partial index
- UNVERIFIED — every wall-clock number in the table (127 to 5.5 ms, 310 to 0.5, and the four others) — a live measurement on a restored store copy with a synthetic year; a read-only tree cannot replay it
- UNVERIFIED — the twice-applied scratch Timescale container, and "with the index an Index Scan, without it a Seq Scan" — needs live PostgreSQL; the second is what the suggested Seq Scan assertion would capture in CI
- UNVERIFIED — Darling.Tests builds with 0 errors — no build was run here
- UNVERIFIED — #6619 and #6635 also claim V215 — those branches are outside this checkout
- UNVERIFIED — the tables hold hundreds of rows on production stores today — a production fact, not a tree fact
- REFUTED — "No lock_timeout on V215. It matches V153." — V153 sets one, at PgMigrations.cs:206, as do V152 (:100), V154 (:219), V173 (:932) and V175 (:1008), and :83-97 records the failure it prevents. The precedent for omitting it is V104, V113 and V207, V207 being a partial index on this very table (ChangeWatchStore.cs:177). The decision is right; the rung cited for it says the opposite
Riskiest lines
- ViewerDataService.cs:1337 — if the sentinel, GetBoolean(190) or the trailing parameter were out of step, a fully-migrated store would probe 214 forever and the connect gate would refuse a current store permanently, no later upgrade changing the answer (CONTRIBUTING.md:277-279) — covered by AgPrimaryReplicaRungTests :54 and :72, live-only
- PgMigrations.cs:2307 — if the condition index's predicate were not implied by WatchedSql's WHERE the index would be dead weight on the 60k-calls-a-day read — covered by AgentTableIndexesV215LiveTests.TheFollowUpModuleRead_UsesTheConditionIndex_AndNotOnceItIsDropped, live-only
- RungRewindArtifacts.cs:515 — a wrong name on the sentinel's drop leaves it on disk and every store-below-rung-N rewind reads 215, the #5641 failure the ladder exists to prevent — covered by AgPrimaryReplicaRungTests:66 and ViewerKnownPatternEditorLivePostgresTests.OnAStoreBelowV214, live-only
Traps near this diff
- "Lite and Darling ship in parity (CONTRIBUTING's two-store rule): a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit" — does not apply — V215 adds no reader, tool, alert, description or state; CONTRIBUTING.md:310 scopes the rule to a rung plus a reader, and Lite/Database/Schema.cs:416-417 records why Lite's investigation tables carry no secondary index at all. The body's reasoning for this is wrong in three places; see the top-level note
- "Ruling (#5833): tests are load-bearing. Never add one, and never answer a trap with a test that reads source text or asserts how code is spelled" — does not apply — the new test EXPLAINs the real read against a seeded store (AgentTableIndexesV215LiveTests.cs:66-67), and the census line (MigrationDataMovingRungCensusPins.cs:332-341) is a declaration an existing required pin demands, not a new census
- "A test that encodes the lie: ACappedResult_WithholdsTheUnsatisfiedSummary_ handed the builder two facets at limit = 2 and asserted truncated - the >= limit inference as its own proof (#3699). Ask what defect a test would fail ON." — applies — AgentTableIndexesV215LiveTests.cs:71 fails on no defect in the rung: no plan can name a dropped index. It proves ExplainAsync re-plans, not that the index is what the measurement rests on. One-line fix suggested inline; non-blocking
- "A test that reads the clock twice measures the CI scheduler (#3676). One anchor owns both ends of an interval." — does not apply — the seed's now() calls (AgentTableIndexesV215LiveTests.cs:58-62) are all transaction_timestamp() inside one statement, so one anchor owns the whole year
- "Our wrong test is a bug we introduced, never a flake: a test that dev's own push broke failed ten unrelated PRs in a week (#5942)." — does not apply — the two tests this rung broke are fixed in its own commits (MigrationDataMovingRungCensusPins.cs:332, ViewerKnownPatternEditorTests.cs:226), and .github/ci-quarantine.txt is untouched
Divergence (blind read vs PR body)
Two places, both in the body's reasoning rather than in the rung.
-
The lock_timeout precedent is inverted. The body offers V153 as the rung that omits one; V153 is one of the five that sets one, and PgMigrations.cs:83-97 records what it prevents - a 264-second aggregate refresh outlasting a flat wait, so the server's retryable 55P03 fires before the client-side command timeout cancels the statement. V207, a partial index on config.investigation_requests itself with no lock_timeout, makes the same point correctly and is one word away.
-
The Lite rationale does not match Lite/. "Lite has the same tables and reads" is not so - no practice_of and no follow_up_condition (Schema.cs:430-465, DuckDbInvestigationStore.cs:146), so indexes 1 and 5 have no Lite counterpart to decline - and the ART-serves-equality argument argues for indexes on Lite's follow_up_of and family_of rather than against them. The reason the tree already holds, that an indexed column an UPDATE writes breaks a second update of the same row in one DuckDB transaction, is correct and stronger. Detail in the top-level note.
Elsewhere the body understates rather than overclaims: it says MigrationProbeLadderTests covers the mapping only, which is exactly what I found - the probe-SQL-to-arity alignment rides entirely on live tests, which is why riskiest line 1 is covered only when DARLING_TEST_PG is set.
Reviewer checklist
- Run the live suite with DARLING_TEST_PG set: AgentTableIndexesV215LiveTests, AgPrimaryReplicaRungTests, ViewerKnownPatternEditorLivePostgresTests.OnAStoreBelowV214, and one rewind chain (CaggGroupIndexDropLiveTests or QueryStoreIntervalPartitionRungTests) to prove no chain now reads 215
- On a restored store copy, confirm the planner picks index 6 inside the fk_investigation_reports_family check: EXPLAIN ANALYZE a retention-shaped DELETE in a rolled-back transaction, a cached SPI plan the suite cannot reach
- Time the six builds on a copy of the largest live store, since no lock_timeout guards the SHARE lock against a long agent transaction on config.investigation_requests
- Settle the rung number against dev at merge time: #6619 and #6635 both claim V215, and a gap is silent and unrepairable (CONTRIBUTING.md:283-293)
Claude posting for Erik Darling. Both divergences were in the body and are corrected: the lock_timeout precedent now cites V207, and the Lite rationale now uses the reason Schema.cs:416-417 records. No code change was needed. The non-blocking Seq Scan assertion goes in a test-only follow-up PR.
Claude posting for Erik Darling.
Adds migration rung V215 with six indexes on the investigation ("agent") tables. A restored store copy loaded with one synthetic year of requests (73,000) at a production store's measured rate timed every hot read, in two runs. Most reads stay under 1 ms. These did not at a year. Each index was tested inside a rolled-back transaction.
ProcedureStatsModuleDates.WatchedSql, about 60k calls a day)DueFollowUpsSql)PendingFollowUpsSqllist_investigation_requests, reports 30-day readfk_investigation_reports_familycheck)The resolve check is fixed separately in #6633 (no rung).
What changed
PgMigrations.cs: V215agent-table-indexes, plainCREATE INDEX IF NOT EXISTS. The tables hold hundreds of rows on production stores today, so each build is instant. Namedix_investigation_requests_reported_condition,_follow_up_of,_follow_up_at,_opened_at,_practice_ofandix_investigation_reports_family_of.StorageVersion.SchemaVersionis 215.pg_indexessentinel on the reports index, a newest-firsthasAgentTableIndexesarm returning 215, and the ladder entry with its drop SQL inRungRewindArtifacts.MigrationProbeLadderTestscovers the mapping only (synthetic booleans). The liveAgPrimaryReplicaRungTestspath runs the real probe SQL and proves a V215 store probes 215. No literal 214 pins existed in the tests.InvestigationTables.CreateSqlis not edited. A fresh store runs every rung and gets the indexes.Predicate check against origin/dev
Each read's WHERE clause implies its index predicate, so the planner can use it. No predicate needed changing.
WatchedSqlhasstate = 'reported'andfollow_up_condition IS NOT NULL.f.follow_up_of = r.request_id, which implies NOT NULL.DueFollowUpsSqlhasstate = 'reported'andfollow_up_at IS NOT NULL.ReportsRequestsSqlandListSqlorder and filter onopened_at.LivePracticeRequestsSqlhaspractice_of IS NOT NULL, and the lookups join onpractice_of =.family_ofis an equality.Tests
There is an established plan-test pattern (
PgStatementTextDueIndexLiveTests), so I added one for the hottest read.AgentTableIndexesV215LiveTests.TheFollowUpModuleRead_UsesTheConditionIndex_AndNotOnceItIsDropped: seeds 73,000 reported requests, EXPLAINsWatchedSql, expects the condition index, then drops it and expects the plan to lose itAgPrimaryReplicaRungTests(existing, live probe)MigrationProbeLadderTestsand the ladder density testsDarling.Tests builds on macOS but can't run there. Locally I applied the exact V215 SQL twice to a scratch Timescale container (idempotent, six indexes). I also ran
WatchedSqlon 73,000 synthetic rows: with the index it plans an Index Scan on it, and without it a Seq Scan. I stopped the container afterwards.Considered, not added
Lite/Database/Schema.cs:416-417). DuckDB runs an UPDATE of an indexed column as a delete and an insert, and a second update of the same row in one transaction trips over it.follow_up_atandfollow_up_conditionare columns an UPDATE writes. Lite'sinvestigation_requestsalso has nopractice_ofand nofollow_up_conditioncolumn, so indexes 1 and 5 have nothing to index there. CONTRIBUTING's parity rule covers a rung plus a reader, and V215 adds no reader.fk_investigation_reports_familycheck using index 6. No trigger: the index leads onfamily_of, the FK column, so an equality probe can use it. Cost is a live PG test I can't run here. Verify in the field.lock_timeouton V215. It follows V207, a partial index onconfig.investigation_requestswith no lock_timeout. The rungs that set one (V152, V153, V154, V173 and V175) lock tables a background refresh policy can hold for about 264 s (PgMigrations.cs:83-97). These are plain config tables that no refresh policy locks. Builds are milliseconds on tables this narrow, and the census pin keeps the 5x lock wait.CHANGELOG entry
CI fix
Two tests failed in the Build run.
MigrationDataMovingRungCensusPins.TheDataMovingRungSet_EqualsTheDeclaredCensus: V215's index builds read rows V184 created, so the scan counts it. I declared V215 ins_declaredwithSetsTheFloor: false. Both tables are narrow control-plane tables (hundreds of rows on a store today, 73,000 in the year-scale test), so each build takes milliseconds.MigrationLockWaitTimeoutSecondsstays at 5x. I did not hide the rung from the scan.ViewerKnownPatternEditorLivePostgresTests.OnAStoreBelowV214...: dropping the V214 table left V215's indexes, so the probe answered 215. The test now also callsRungRewindArtifacts.DropSentinelsAboveAsync(connection, 214, ct), as the other below-Vn tests do. No other test drops a V214 sentinel and then probes (the brief reader test does not probe).Review round
Review round 2 (review at f9b923e)
Schema.cs:416-417already records.