Skip to content

Commit bb7cb41

Browse files
os-zhuangclaude
andauthored
fix(metadata): sys_view_definition active-row index gets a NULL-safe key (#6417) (#6666)
SQL UNIQUE treats NULLs as mutually distinct, so `(name, organization_id, owner)` constrained PERSONAL views only: `owner` is NULL for SHARED views and `organization_id` is NULL for environment-level ones, and two same-name active shared views could coexist inside one tenant while `name` is declared as the globally unique qualified view id. Per the maintainer ruling of 2026-08-08 that is now forbidden. The runtime migration #5839 introduced materializes the key NULL-safe, copying one in-repo precedent per column class rather than inventing a form: ADR-0120 D3's `COALESCE(organization_id, '__global__')` for the tenant column, and `ensureOverlayIndex`'s `COALESCE(package_id, '')` form as `COALESCE(owner, '')` for the non-tenant discriminator. Storage is untouched. Unlike #5839 this is a tightening and can fail to build on existing data, so the conflict branch is now a live path and takes ADR-0120 D4's disposition in full: keep the previous index, name the key that is not enforced, ship the exact query that lists the offending rows, point at `os migrate plan`, never block boot. The degradation reports are raised from `info`/`warn` to `error`: what the missing DDL costs changed in kind, from slot recycling (functional, the next user finds out) to an integrity guarantee the platform states it enforces while continuing to look healthy — AGENTS.md's durability arm. PR #6415's honest pin (`does NOT close the pre-existing NULL-distinct hole for shared views`) is flipped to the positive assertion. Claude-Session: https://claude.ai/code/session_01W6bLax4KMrSfnE1ydFU8Dw Co-authored-by: Claude <noreply@anthropic.com>
1 parent b3efeb7 commit bb7cb41

4 files changed

Lines changed: 531 additions & 69 deletions

File tree

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
"@objectstack/metadata-core": patch
4+
---
5+
6+
fix(metadata): two same-name active SHARED views can no longer coexist — `sys_view_definition`'s active-row index gets a NULL-safe key (#6417)
7+
8+
#5839 / PR #6415 delivered "unique among ACTIVE rows" for `sys_view_definition`
9+
as a runtime partial UNIQUE index, and deliberately changed only the index's
10+
**row scope** — that is what made it strictly weaker than the index it replaced
11+
and therefore incapable of failing on existing data. It also left the other
12+
half of the same index broken, and pinned that gap honestly rather than closing
13+
it.
14+
15+
SQL UNIQUE treats NULLs as mutually **distinct**. `owner` is NULL for SHARED
16+
views and `organization_id` is NULL for environment-level ones, so
17+
`(name, organization_id, owner)` constrained **personal views only**. Measured
18+
on real SQLite over the driver's own DDL:
19+
20+
```text
21+
two ACTIVE personal views, same (name, org, owner) : REJECTED
22+
two ACTIVE shared views (owner NULL) : OK ← unconstrained
23+
two ACTIVE env-level views (organization_id NULL) : OK ← unconstrained
24+
```
25+
26+
Two same-name shared views inside one tenant were therefore reachable, while
27+
`name` is declared as the globally unique qualified view id (`object.viewKey`)
28+
— so the view switcher, which aggregates and de-duplicates by `name`, and every
29+
read path that locates a view by name, had no defined answer about which row
30+
they got.
31+
32+
**What changes.** Per the maintainer ruling of 2026-08-08 this is now forbidden.
33+
The same runtime migration materializes the key NULL-safe, folding each nullable
34+
part's NULLs into one bucket that is unique among itself:
35+
36+
```sql
37+
CREATE UNIQUE INDEX idx_sys_view_def_active ON sys_view_definition
38+
(name, COALESCE(organization_id, '__global__'), COALESCE(owner, ''))
39+
WHERE state = 'active'
40+
```
41+
42+
Both spellings are copied from an existing in-repo precedent rather than
43+
invented: `'__global__'` is ADR-0120 D3's reserved sentinel for the tenant
44+
column (the driver's `GLOBAL_TENANT`), and `COALESCE(owner, '')` is
45+
`ensureOverlayIndex`'s `COALESCE(package_id, '')` form for a non-tenant nullable
46+
discriminator. Neither can collide with real data — an organization id may never
47+
equal `'__global__'`, and an owner is a user id, never the empty string.
48+
**Storage is untouched**: rows keep their NULLs, only the index folds them, so
49+
`WHERE owner = ''` still matches nothing.
50+
51+
Unchanged: archived rows stay exempt (#5839's active-only scoping survives, on
52+
shared views too), a shared view and a personal view may still share a name, and
53+
so may two tenants' or two environments' rows.
54+
55+
**This is a tightening, so it can fail to build.** Unlike #5839, rows that
56+
violate the new key exist in the wild today, precisely because nothing rejected
57+
them. The migration probes before it replaces anything, and on a conflict takes
58+
ADR-0120 D4's disposition: the previous index is left in place (the table is
59+
never left unconstrained), the report names the key that is not enforced, ships
60+
the exact `GROUP BY … HAVING COUNT(*) > 1` query that lists the offending rows,
61+
points at `os migrate plan` — and the boot continues. Resolve the duplicate
62+
active shared views, restart, and the tightening applies itself.
63+
64+
Dialects with no partial indexes (MySQL/MariaDB) keep the declared bare
65+
composite, which is ADR-0120 D3's own degradation. That report is **raised from
66+
`info` to `error`**: under #5839 alone the dialect lost slot recycling, a
67+
functional degradation the next user hits immediately, but it now loses an
68+
integrity guarantee the platform states it enforces while continuing to look
69+
healthy — AGENTS.md's durability arm. The line names both gaps that stay open
70+
there and the duplicate-listing query. The unclassifiable-failure arm is raised
71+
with it, so the failure nobody can name is never reported more quietly than the
72+
one that has a name.

packages/metadata-core/src/objects/sys-view-definition.object.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,8 @@ export const SysViewDefinitionObject = ObjectSchema.create({
120120

121121
indexes: [
122122
// A given view name is unique per (organization, owner) — a shared view
123-
// (owner NULL) and each user's personal views don't collide.
123+
// (owner NULL) and each user's personal views don't collide, AND two
124+
// shared views may not share a name either (#6417).
124125
//
125126
// ⚠️ This entry is the FALLBACK shape, not the delivered one. It carried
126127
// `partial: "state = 'active'"` until #5248 / #4943 retired the key,
@@ -139,10 +140,27 @@ export const SysViewDefinitionObject = ObjectSchema.create({
139140
// `syncDeclaredIndexes` (which skips by name) never re-imposes the
140141
// unrestricted form on a later boot.
141142
//
143+
// ⚠️ The KEY below is NULL-DISTINCT, which is a second gap the declaration
144+
// cannot close on its own (#6417). `owner` is NULL for SHARED views and
145+
// `organization_id` is NULL for environment-level ones, and SQL UNIQUE
146+
// treats NULLs as mutually distinct — so what this entry constrains is
147+
// PERSONAL views only, measured: two active shared views could carry one
148+
// name. Per the maintainer ruling of 2026-08-08 that is forbidden, and the
149+
// same runtime migration delivers it, again without touching this
150+
// declaration: it materializes the key NULL-safe, as
151+
// `(name, COALESCE(organization_id, '__global__'), COALESCE(owner, ''))`
152+
// — ADR-0120 D3's sentinel for the tenant column, `ensureOverlayIndex`'s
153+
// `COALESCE(package_id, '')` form for the non-tenant one. Storage keeps
154+
// its NULLs; only the index folds them into a bucket.
155+
//
142156
// Keep this declaration exactly as it is. It is what dialects without
143157
// partial indexes (MySQL) and hosts that never run the migration fall back
144158
// to, and the migration deliberately leaves it untouched when it cannot
145-
// build the partial form — degraded to this behaviour, never below it.
159+
// build the partial NULL-safe form — degraded to this behaviour, never
160+
// below it. Rewriting it to `unique: 'organization'` would NOT be the same
161+
// thing: that is ADR-0120 D1's declared-scope vocabulary, staged for the
162+
// protocol-18 train (D7), and it scopes the tenant column only — `owner`
163+
// would stay NULL-distinct.
146164
{
147165
name: 'idx_sys_view_def_active',
148166
fields: ['name', 'organization_id', 'owner'],

0 commit comments

Comments
 (0)