fix(driver-sql): a failed index read is an error, not an empty index list (#7332) - #7394
Merged
Merged
Conversation
…list (#7332) `SqlDriver.introspectIndexes` wrapped its entire dialect dispatch — SQLite, Postgres and MySQL alike — in one bare `catch {}` and returned its accumulator half-built. Callers could not tell "this table genuinely has no such index" from "the read failed and I am guessing". `diffManagedIndexes` takes its declared-index-missing branch on exactly that input, so a transient SQLITE_BUSY or a WAL read landing mid-flush became a confident, specific and false `actual: '(absent)'` report about an index that was there the whole time. Split the swallow by call site rather than removing it. Its justification — "let creation handle conflicts" — holds at `getExistingIndexNames`, whose caller `syncDeclaredIndexes` corrects an optimistic wrong reading by attempting the create and absorbing "already exists"; a throw there would take a boot down on a transient read. Detection has no such backstop and inherited the swallow only because #3728 wired a second consumer onto the same function. `introspectIndexes` now throws by default and takes an explicit `{ onFailure: 'partial' }` opt-in that only the creation seam passes. The detection callers were already built for this: `reconcileAndWarnDrift` catches and warns "could not introspect '<table>' for drift detection", and `os migrate plan` / `apply` both catch, print and exit non-zero. The sibling read in the same detect path, `introspectColumns`, has never swallowed. Measured: no consumer ever acted destructively on the false reading. Dropping entries from the physical list is monotone — `replace_unique_index`, `drop_index` and `recreate_index` all require an index to be present — so a short read can only remove a destructive proposal, never arm one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Wv1i1AwBy8eETqaDCXV6B
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 8 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
documentation
os-zhuang
marked this pull request as ready for review
August 10, 2026 09:53
Contributor
⛔ merge queue 构建失败 — 先分诊，再决定要不要重排队列构建 31377184925 红了。队列跑的是全量套件（PR 侧 CI 只跑 affected 子集）， 失败的 job（日志抽取，best effort）：
历史信号：
分诊清单：
Generated by Claude Code · merge-queue-triage workflow (#4859) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #7332.
What was wrong
SqlDriver.introspectIndexeswrapped its entire dialect dispatch — the SQLite, Postgres and MySQL branches alike — in one barecatch {}and returned its accumulator in whatever half-built state it had reached. The caller could not tell "this table genuinely has no such index" from "the read failed and I am guessing".Drift detection consumes that same function.
diffManagedIndexestakes its declared-index-missing branch on exactly that input, so a transient failure was not surfaced as an error — it was laundered into a confident, specific and false report. Measured on this branch before the fix, by failingPRAGMA index_listwithSQLITE_BUSYagainst a table whose declared index demonstrably exists:{ "kind": "index_mismatch", "table": "product", "expected": "(code)", "actual": "(absent)", "severity": "warning", "category": "safe", "op": { "type": "create_index", "indexName": "idx_product_code", "columns": ["code"] }, "message": "product: metadata declares index 'idx_product_code' (code) but the database has no such index — run \"os migrate apply\" to create it." }What changed
packages/drivers/driver-sql/src/sql-driver.tsonly — the swallow is split by call site, not removed.introspectIndexes(tableName, opts?)now throws by default, and takes{ onFailure: 'partial' }for a caller that can correct a short read.getExistingIndexNames— the creation seam — passes{ onFailure: 'partial' }. Nothing else does.Its return type is unchanged, so
sql-driver-overlay-index-drift.test.ts:194(which types the direct call asPhysicalIndex[]) still compiles and passes untouched.Why the swallow stays on creation
Its stated justification — "let creation handle conflicts" — is sound exactly where it was written.
syncDeclaredIndexescallsgetExistingIndexNameson its first line, outside any try: a throw there takes the whole boot down on a transient read. And the backstop is real and verified — the create is attempted and the duplicate absorbed by the/already exists|duplicate key name|exists/ibranch in the same function.Detection has no such backstop, and inherited the swallow only because #3728 wired a second consumer onto a function written for the first. Its callers were already built for a throw:
reconcileAndWarnDriftwrapsdetectTableDriftincatch → logger.warn("could not introspect '<table>' for drift detection") → return. That handler was reachable only viaintrospectColumns, which has never swallowed — the index dimension was the odd one out in its own detect path.os migrate planandos migrate applybothcatch → printError → this.exit(1).Not a destructive defect — measured, and pinned
No consumer ever acted destructively on the false
(absent). Dropping entries from the physical list is monotone:replace_unique_indexneeds the legacy index present,drop_indexneeds the orphan present,recreate_indexneeds the declared name present. A short read can therefore only remove a destructive proposal, never arm one. The last case in the new test pins that direction rather than leaving it as prose.Tests
New file
sql-driver-index-introspection-failure.test.ts(8 cases). Reverse-verified: 4 predicted red, 4 predicted green as controls — measured exactly 4/4 before the fix, 8/8 after.Relationship to #6522 — unproven
#6522 closed as not reproduced in 67 runs. This is a better-shaped hypothesis for that flake, not a demonstrated cause — nobody has reproduced it, and nothing here claims to fix it.
sql-driver-overlay-index-drift.test.tsis untouched by this PR.Generated by Claude Code