Skip to content

Commit 32fa1ad

Browse files
committed
feat(driver-sql)!: NULL-safe organization uniques + declared-index 'organization' scope (ADR-0120 D3/D4, #5030)
Every organization-scoped unique index now materializes its organization key part as COALESCE(organization_id, '__global__') instead of the bare column: SQL UNIQUE is NULL-distinct, so the #3696 composite enforced nothing on NULL-organization rows — on a single-tenant stack, every row (#5030, measured). - uniqueIndexesFromFields / legacy replacements emit the NULL-safe key part; field-level unique: 'organization' accepted as the explicit synonym of true. - normalizeDeclaredIndex / expectedIndexes / syncDeclaredIndexes learn declared unique: 'organization' (prepend the org key part; degrade with no tenant column; a listed tenant column is made NULL-safe in place). 'global' / bare true stays VERBATIM — the #3696 contract, now the 'global' arm (spec token lands separately via #4986; driver deliberately merges first). - Drift both sides share one normalization: physical COALESCE(col, <literal>) attributes to col, compared literal-agnostically; the org key part is the sync's own vocabulary (isSyncReproducibleIndex + tenantField), scoped so the ADR-0048 overlay indexes keep their #4884 protection. - D4 ceremony: the bare-composite tightening is a recreate_index gated by a duplicate pre-flight probe — clean → safe (dev autoMigrate applies), dirty → BLOCKED with a row report, old index kept; apply re-probes so even --allow-destructive cannot drop a constraint whose replacement fails. - plugin-auth: '__global__' reserved as organization id/slug at the beforeCreateOrganization seam (ADR-0120 D3 guardrail). - Tests per ADR-0120 D6.6: contract header rewritten, #5030 probe graduated as a permanent regression, NULL-bucket semantics, declared 'organization' pins, pre-flight both states; 'exactly as authored' retained for 'global'. - ADR anchors for schema-drift.ts / sql-driver.ts (PD #13). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Akrzh2mHi2siSNVPPtfTw7
1 parent e96ad55 commit 32fa1ad

10 files changed

Lines changed: 1225 additions & 131 deletions
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
---
2+
"@objectstack/driver-sql": minor
3+
"@objectstack/plugin-auth": patch
4+
---
5+
6+
feat(driver-sql)!: organization-scoped uniques are NULL-safe — `COALESCE(organization_id, '__global__')` key part + `unique: 'organization'` on declared indexes (ADR-0120 D3/D4, #5030)
7+
8+
SQL UNIQUE is NULL-distinct, so the `(organization_id, field)` composite #3696
9+
introduced enforced **nothing** on rows whose organization is NULL — which on a
10+
single-tenant stack (where the kernel injects the column and never fills it) is
11+
**every row**: field-level `unique: true` was a silent no-op there, measured in
12+
#5030. Per ADR-0120 D3, every organization-scoped unique now materializes its
13+
organization key part as `COALESCE(organization_id, '__global__')`: NULL-organization
14+
rows collapse into one platform bucket, unique among themselves; non-NULL rows
15+
are untouched. Storage stays NULL — the sentinel exists only inside the index
16+
key, and it is the same word the autonumber sequence table already uses
17+
(`GLOBAL_TENANT`), so a constraint-violation error reads as "the platform
18+
bucket collided", not as corrupt data.
19+
20+
What changes, concretely:
21+
22+
- **Field-level `unique: true`** (and the new explicit synonym
23+
`'organization'`) on a tenant-scoped object → composite
24+
`(COALESCE(tenantField, '__global__'), field)`. `unique: 'global'` and
25+
tenant-less objects are unchanged.
26+
- **Declared indexes gain the ADR-0120 D1 scope vocabulary at the driver**:
27+
`unique: 'organization'` prepends the NULL-safe organization key part to the
28+
listed columns (degrading to the listed columns on a tenant-less object; a
29+
listed tenant column is made NULL-safe in place instead — the S6 respelling).
30+
`unique: true` / `'global'` on a declared index stays **verbatim** — the
31+
#3696 contract, now the `'global'` arm; the nine engine dedup/idempotency
32+
keys keep their exact physical shape. (The spec/lint side of the vocabulary
33+
lands separately via #4986; the driver deliberately merges first.)
34+
- **Drift detection reads both sides through one normalization**
35+
(the #4884 discipline, extended to the tenant key part): the physical
36+
`COALESCE(organization_id, <literal>)` form is attributed to the column,
37+
compared **literal-agnostically**, and recognised as the sync's own
38+
vocabulary — a healthy database reports zero drift on every dialect.
39+
- **Existing bare composites migrate through the ceremony (ADR-0120 D4)**:
40+
`(organization_id, X) → (COALESCE(organization_id, '__global__'), X)`
41+
surfaces as a `recreate_index` drift op — a pure tightening — gated by a
42+
**duplicate pre-flight probe**. Clean probe → the op grades `safe` and dev
43+
`autoMigrate: 'safe'` / a plain `os migrate apply` applies it. Duplicates
44+
(data the void constraint wrongly admitted) → the op is **blocked** with a
45+
per-group row report, the old index stays in place, and apply re-probes so
46+
even `--allow-destructive` cannot drop a constraint whose replacement is not
47+
creatable. Deduplicate, re-plan, apply.
48+
- **`'__global__'` is reserved at the organization-minting seam**
49+
(plugin-auth): an organization whose id or slug equals the sentinel is
50+
rejected at creation with a prescriptive error (ADR-0120 D3 guardrail).
51+
52+
Migration note for operators: on databases with pre-existing
53+
organization-composite uniques, the first `os migrate plan` after upgrading
54+
shows one `recreate_index` per affected index. On healthy data it auto-applies
55+
in dev and is a no-op content-wise; a blocked op means the #5030 defect
56+
admitted real duplicate rows — resolve the listed rows first. MySQL < 8.0.13 /
57+
MariaDB cannot express the functional key part: the driver degrades to the
58+
bare composite, says exactly what is not enforced at `error` level, and keeps
59+
reporting the tightening as drift for after the server upgrade.

packages/plugins/driver-sql/src/index.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,14 @@ export {
3434
parseIndexDdl,
3535
uniqueIndexesFromFields,
3636
INDEX_DRIFT_OPS,
37+
// Unique-scope vocabulary + NULL-safe organization key part (ADR-0120 D1/D3)
38+
GLOBAL_TENANT,
39+
isUniqueScopeDeclared,
40+
isOrganizationScopedUnique,
41+
organizationKeyPartSql,
3742
} from './schema-drift.js';
3843
export type {
44+
DeclaredIndexInput,
3945
ManagedDriftEntry,
4046
DriftOp,
4147
DriftCategory,

packages/plugins/driver-sql/src/schema-drift.ts

Lines changed: 291 additions & 52 deletions
Large diffs are not rendered by default.

packages/plugins/driver-sql/src/sql-driver-index-drift.test.ts

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import { describe, it, expect, afterEach, vi } from 'vitest';
4-
import { SqlDriver, diffManagedIndexes, isManagedIndexName } from '../src/index.js';
4+
import {
5+
SqlDriver,
6+
classifyIndexKeyPart,
7+
diffManagedIndexes,
8+
isManagedIndexName,
9+
parseIndexDdl,
10+
} from '../src/index.js';
511

612
/**
713
* Index-dimension managed-schema drift (#3728).
@@ -65,13 +71,37 @@ describe('SqlDriver index drift (#3728)', () => {
6571
},
6672
];
6773

74+
/**
75+
* Unique index name → canonical key parts, read from the index DDL so
76+
* expression keys are visible (`PRAGMA index_info` reports a NULL name for
77+
* them). A plain column reads as its name; the NULL-safe organization key
78+
* part (ADR-0120 D3) reads as `COALESCE(<column>)` — literal elided, the
79+
* same literal-agnostic identity the drift differ compares on.
80+
*/
6881
const uniqueIndexColumns = async (table: string): Promise<Record<string, string[]>> => {
6982
const list: any = await knexInstance.raw(`PRAGMA index_list(${table})`);
83+
const master: any = await knexInstance.raw(
84+
`SELECT name, sql FROM sqlite_master WHERE type = 'index' AND tbl_name = ?`,
85+
[table],
86+
);
87+
const ddlByName = new Map<string, string>();
88+
for (const r of Array.isArray(master) ? master : (master?.rows ?? [])) {
89+
if (typeof r?.sql === 'string' && r.sql) ddlByName.set(r.name, r.sql);
90+
}
7091
const out: Record<string, string[]> = {};
7192
for (const idx of list) {
7293
if (idx.origin === 'pk' || idx.unique !== 1) continue;
73-
const info: any = await knexInstance.raw(`PRAGMA index_info("${idx.name}")`);
74-
out[idx.name] = info.map((c: any) => c.name);
94+
const parsed = parseIndexDdl(ddlByName.get(idx.name) ?? '');
95+
if (parsed) {
96+
out[idx.name] = parsed.keyParts.map((p) => {
97+
const part = classifyIndexKeyPart(p);
98+
if (part.kind === 'column') return part.column;
99+
return part.column === null ? p : `COALESCE(${part.column})`;
100+
});
101+
} else {
102+
const info: any = await knexInstance.raw(`PRAGMA index_info("${idx.name}")`);
103+
out[idx.name] = info.map((c: any) => c.name);
104+
}
75105
}
76106
return out;
77107
};
@@ -185,7 +215,7 @@ describe('SqlDriver index drift (#3728)', () => {
185215
// Both are current intent: the tenant composite from the field-level
186216
// `unique: true`, and the verbatim declared global unique.
187217
const uniques = await uniqueIndexColumns('hp_contact');
188-
expect(uniques['uniq_hp_contact_organization_id_email']).toEqual(['organization_id', 'email']);
218+
expect(uniques['uniq_hp_contact_organization_id_email']).toEqual(['COALESCE(organization_id)', 'email']);
189219
expect(uniques['uniq_hp_contact_email']).toEqual(['email']);
190220

191221
// Before the fix this reported `replace_unique_index` — proposing to drop
@@ -260,7 +290,7 @@ describe('SqlDriver index drift (#3728)', () => {
260290

261291
const uniques = await uniqueIndexColumns('product');
262292
expect(uniques['product_code_unique']).toBeUndefined();
263-
expect(Object.values(uniques)).toContainEqual(['organization_id', 'code']);
293+
expect(Object.values(uniques)).toContainEqual(['COALESCE(organization_id)', 'code']);
264294

265295
// Existing rows survived, and the cross-tenant insert the issue is about works.
266296
expect(await driver.count('product', { object: 'product' })).toBe(2);
@@ -281,7 +311,7 @@ describe('SqlDriver index drift (#3728)', () => {
281311
const again = await driver.applyMigrationEntries(drift, { allowDestructive: false });
282312
expect(again.skipped).toHaveLength(0);
283313
expect(Object.values(await uniqueIndexColumns('product'))).toContainEqual([
284-
'organization_id',
314+
'COALESCE(organization_id)',
285315
'code',
286316
]);
287317
});
@@ -298,7 +328,7 @@ describe('SqlDriver index drift (#3728)', () => {
298328

299329
const uniques = await uniqueIndexColumns('product');
300330
expect(uniques['product_code_unique']).toBeUndefined();
301-
expect(Object.values(uniques)).toContainEqual(['organization_id', 'code']);
331+
expect(Object.values(uniques)).toContainEqual(['COALESCE(organization_id)', 'code']);
302332
expect(await driver.detectManagedDrift()).toHaveLength(0);
303333

304334
const b = await driver.create('product', { organization_id: 'org_b', code: 'PROD-00001' });

packages/plugins/driver-sql/src/sql-driver-overlay-index-drift.test.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,4 +357,91 @@ describe('overlay index drift on a fresh database (#4884)', () => {
357357
expect(index.expressions).toEqual(["COALESCE(package_id, '')", 'lower(name)']);
358358
expect(isSyncReproducibleIndex(index)).toBe(false);
359359
});
360+
361+
// ── ADR-0120 D3: the NULL-safe organization key part in the drift reader ──
362+
363+
describe('NULL-safe organization key part attribution (ADR-0120 D3)', () => {
364+
it("attributes COALESCE(organization_id, '__global__') to organization_id in every dialect spelling", () => {
365+
for (const sql of [
366+
// SQLite: `sqlite_master.sql`, as the sync wrote it.
367+
"COALESCE(organization_id, '__global__')",
368+
'COALESCE("organization_id", \'__global__\')',
369+
// Postgres: `pg_get_indexdef` casts a varchar key part.
370+
"COALESCE((organization_id)::text, '__global__'::text)",
371+
// MySQL: `information_schema.STATISTICS.EXPRESSION` decorates the
372+
// literal with a charset introducer and backslash-escaped quotes.
373+
"coalesce(`organization_id`,_utf8mb4\\'__global__\\')",
374+
]) {
375+
expect(classifyIndexKeyPart(sql)).toEqual({
376+
kind: 'expression',
377+
sql,
378+
column: 'organization_id',
379+
});
380+
}
381+
});
382+
383+
it('applyIndexKeyParts records the attributed key part as a NULL-safe column', () => {
384+
const index: PhysicalIndex = { name: 'i', columns: [], unique: true };
385+
applyIndexKeyParts(index, ["COALESCE(organization_id, '__global__')", 'email']);
386+
expect(index.columns).toEqual(['organization_id', 'email']);
387+
expect(index.nullSafeColumns).toEqual(['organization_id']);
388+
});
389+
390+
it("isSyncReproducibleIndex: the org key part is the sync's OWN vocabulary — for the tenant column only", () => {
391+
const org: PhysicalIndex = { name: 'i', columns: [], unique: true };
392+
applyIndexKeyParts(org, ["COALESCE(organization_id, '__global__')", 'email']);
393+
expect(isSyncReproducibleIndex(org, 'organization_id')).toBe(true);
394+
expect(isSyncReproducibleIndex(org, null)).toBe(false);
395+
expect(isSyncReproducibleIndex(org)).toBe(false);
396+
// The ADR-0048 overlay key attributes to a NON-tenant column and stays
397+
// out — loosening the column scoping to "any attributable COALESCE"
398+
// would resurrect the #4884 false orphan on the overlay indexes.
399+
const overlay: PhysicalIndex = { name: 'o', columns: [], unique: true };
400+
applyIndexKeyParts(overlay, ['type', "COALESCE(package_id, '')"]);
401+
expect(isSyncReproducibleIndex(overlay, 'organization_id')).toBe(false);
402+
});
403+
404+
it('the differ compares key-part FORM literal-agnostically: COALESCE ≡ COALESCE, bare ≠ COALESCE', () => {
405+
const expected = [
406+
{
407+
name: 'uniq_t_organization_id_email',
408+
columns: ['organization_id', 'email'],
409+
unique: true,
410+
nullSafeColumns: ['organization_id'],
411+
},
412+
];
413+
// Physical carries the COALESCE form with a DIFFERENT literal: any
414+
// literal folds NULL into one bucket, so it is the same constraint —
415+
// zero drift (the #4884 lesson applied to the tenant key part).
416+
const physSame: PhysicalIndex = { name: 'uniq_t_organization_id_email', columns: [], unique: true };
417+
applyIndexKeyParts(physSame, ["COALESCE(organization_id, '')", 'email']);
418+
expect(
419+
diffManagedIndexes({
420+
table: 't',
421+
expected,
422+
legacy: [],
423+
physical: [physSame],
424+
tenantField: 'organization_id',
425+
}),
426+
).toEqual([]);
427+
// Physical is the bare NULL-distinct composite: a DIFFERENT constraint
428+
// (void on NULL-organization rows, #5030) → the D4 tightening op.
429+
const physBare: PhysicalIndex = {
430+
name: 'uniq_t_organization_id_email',
431+
columns: ['organization_id', 'email'],
432+
unique: true,
433+
};
434+
const out = diffManagedIndexes({
435+
table: 't',
436+
expected,
437+
legacy: [],
438+
physical: [physBare],
439+
tenantField: 'organization_id',
440+
});
441+
expect(out).toHaveLength(1);
442+
expect(out[0].op).toMatchObject({ type: 'recreate_index', tightenNullSafeOnly: true });
443+
expect(out[0].expected).toBe("UNIQUE (COALESCE(organization_id, '__global__'), email)");
444+
expect(out[0].actual).toBe('UNIQUE (organization_id, email)');
445+
});
446+
});
360447
});

0 commit comments

Comments
 (0)