Skip to content

Commit bed427f

Browse files
os-zhuangclaude
andauthored
fix(metadata-protocol): ensureOverlayIndex probes before it drops, and reports what it could not enforce (#6418) (#6770)
* fix(metadata-protocol): ensureOverlayIndex probes before it drops, and reports what it could not enforce (#6418) `sys_metadata`'s overlay-uniqueness migration ran DROP-then-CREATE with nothing restoring the dropped index and both catch blocks empty, so on SQLite/PostgreSQL a CREATE that failed on existing rows left the table with no unique index at all and no line in the log. The degradation branch only fired on /partial|where clause|syntax/i, which duplicate-row errors never match. The order is now probe-first — build under a throwaway name, and only once that has succeeded drop the real name and rebuild it — extracted into a shared partial-index-probe.ts that view-definition-active-index.ts also uses (its behaviour and tests are unchanged). Both overlay sections (active and draft) get the treatment, independently. The empty catches are replaced by ADR-0120 D4's disposition: keep the previous index, name the unenforced key, ship the duplicate-listing query, point at `os migrate plan`, never block boot. The key spelling is byte-identical and the dialect fallback stays non-UNIQUE (an active row and a draft row for one key legitimately coexist). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W6bLax4KMrSfnE1ydFU8Dw * fix(metadata-protocol): drop the unused dropIndexQuietly import (#6418) The type-check debt ratchet caught it: 64 vs the recorded 63. `overlay-index.ts` issues no drops of its own — `probeThenReplaceIndex` owns them all, and the dialect fallback is deliberately additive. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W6bLax4KMrSfnE1ydFU8Dw --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f6cd635 commit bed427f

7 files changed

Lines changed: 1391 additions & 164 deletions

File tree

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
fix(metadata-protocol): `ensureOverlayIndex` probes before it drops, and says what it could not enforce (#6418)
6+
7+
`sys_metadata`'s overlay-uniqueness migration ran **DROP then CREATE**:
8+
9+
```text
10+
DROP INDEX IF EXISTS idx_sys_metadata_overlay_active ← always succeeds
11+
CREATE UNIQUE INDEX idx_sys_metadata_overlay_active … ← may fail
12+
```
13+
14+
with nothing that puts the dropped index back, and both `catch` blocks empty. On
15+
the dialects that *do* support the form (SQLite / PostgreSQL), a `CREATE` that
16+
failed on existing rows therefore left the table with **no** unique index at all
17+
— and no line in the log. ADR-0005 overlay uniqueness is the base of metadata
18+
correctness: with two ACTIVE rows for one
19+
`(type, name, organization_id, package_id)`, which one `getMetaItem` returns is
20+
undefined.
21+
22+
The degradation branch could not save it either. It fired only when the driver's
23+
message matched `/partial|where clause|syntax/i`, which duplicate-row errors
24+
(`UNIQUE constraint failed` / `duplicate key value`) do not — so the one failure
25+
that is about DATA fell through to a bare `// best-effort` comment. MySQL was
26+
safe only by accident: `DROP INDEX IF EXISTS` is not legal MySQL, so the drop
27+
failed first and the old index survived.
28+
29+
**The order is now probe-first**, ported from the sibling
30+
`view-definition-active-index.ts` (#5839 / #6417) and extracted into a shared
31+
`partial-index-probe.ts` both migrations use: build the partial UNIQUE under a
32+
throwaway probe name, and only once that has demonstrably succeeded drop the
33+
real name and rebuild it. On any dialect or dataset that cannot take the form,
34+
whatever index was protecting the table is left exactly as it was — degraded to
35+
yesterday's behaviour, never below it. Both sections get this treatment
36+
(`…_overlay_active` and `…_overlay_draft`), and the two are independent so a
37+
failure on one no longer decides the other.
38+
39+
**The empty catches are replaced by ADR-0120 D4's disposition**: classify the
40+
failure, keep the previous index, name the key that is not enforced and what
41+
that costs, ship the exact query that lists the offending rows, point at
42+
`os migrate plan`, and let the boot continue — reported at `error`, because what
43+
goes missing is an integrity guarantee the platform states it enforces while
44+
everything else keeps looking healthy.
45+
46+
Two things deliberately do **not** change. The key spelling stays byte-identical
47+
(`(type, name, organization_id, COALESCE(package_id, ''))`) — this is an
48+
ordering and reporting fix, not a re-keying. And the dialect fallback stays a
49+
**non-UNIQUE** composite index: one ACTIVE row and one DRAFT row for the same
50+
key legitimately coexist on this table, so a full UNIQUE would reject legal
51+
data. What changes about the fallback is that it is now issued additively
52+
(`IF NOT EXISTS`, no preceding drop, so it can never replace a stronger index)
53+
and that the report says plainly what is and is not enforced.

packages/metadata-protocol/src/migrations/overlay-index.test.ts

Lines changed: 529 additions & 0 deletions
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)