Skip to content

Commit cd13488

Browse files
os-zhuangclaude
andauthored
fix(driver-sql): a shadow-carried UNIQUE is not index drift, and its remedy dropped the constraint (#13055)
The index differ compared declared columns against the columns an index physically KEYS. A #11627 hash-shadow-carried UNIQUE keys exactly one driver-owned VARBINARY(32) generated column, so the comparison could never match and a clean boot reported the index it had just created as destructive `recreate_index` drift. Following that remedy removed the constraint: the drop-by-name leaves the generated column behind, the re-sync's shadow `ADD COLUMN` fails on the survivor with a duplicate-COLUMN error that neither the "already exists" absorb nor the unique-violation branch matches, and the apply ends with the UNIQUE dropped and not re-created. Both passes now read one vocabulary instead of special-casing the differ: the shadow name derivation moves next to `isHashShadowColumn`, introspection resolves what the shadow HASHES from the stored GENERATION_EXPRESSION, the differ compares the ENFORCED key, and the sync inspects a surviving shadow column (re-key / re-generate / refuse) rather than assuming it absent. A real key comparison, not a blanket skip: a pre-#12998 shadow hashes the RAW columns and leaves every NULL-organization row unconstrained, and is indistinguishable by name from a healthy one. It stays reported, as the ADR-0120 D4 tightening it is. Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry Co-authored-by: Claude <noreply@anthropic.com>
1 parent ef744c4 commit cd13488

4 files changed

Lines changed: 778 additions & 28 deletions

File tree

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
'@objectstack/driver-sql': patch
3+
---
4+
5+
A healthy hash-shadow-carried UNIQUE is no longer reported as destructive index
6+
drift — and the remedy that used to be proposed for it would have DROPPED the
7+
constraint
8+
9+
On MySQL a declared UNIQUE whose key is too wide for an InnoDB key part is
10+
carried by a driver-owned generated column holding a SHA-256 of the key values
11+
(#11627). The index differ compared the declared columns against the columns an
12+
index physically KEYS, so a shadow-carried UNIQUE — one VARBINARY(32) generated
13+
column as the whole key — could never match. A clean `initObjects` reported the
14+
index the same boot had just created as `index_mismatch` / `destructive`, with
15+
`recreate_index` as the remedy and `os migrate apply --allow-destructive` in the
16+
message.
17+
18+
Following that advice removed a live uniqueness guarantee. `recreate_index`
19+
drops the UNIQUE by name and re-runs the additive sync; the sync retakes the
20+
shadow route, and its `ALTER TABLE … ADD COLUMN` then failed on the generated
21+
column that **survived** the index drop. That failure is a duplicate-COLUMN
22+
error, matched by neither the "already exists" absorb (which spells index names)
23+
nor the unique-violation branch — so the apply ended with the constraint dropped
24+
and not re-created.
25+
26+
Both halves are fixed, and they share one vocabulary rather than special-casing
27+
the differ. The orphan-COLUMN pass already recognised the shadow as driver-owned
28+
(`isHashShadowColumn`) while the index it carries was proposed for destructive
29+
rebuild; that asymmetry was the shape of the defect.
30+
31+
- The shadow's name derivation moved next to that predicate, so the name the
32+
sync creates and the name the differ looks for have one definition.
33+
- Introspection reads the shadow's stored `GENERATION_EXPRESSION` and records
34+
the key it actually hashes, so the differ compares the key the constraint
35+
**enforces** instead of the digest column it stores. Drift reports and plan
36+
messages now name that key too, rather than `UNIQUE (uniq_…__hash)`.
37+
- The sync inspects a surviving shadow column instead of assuming it absent: a
38+
column already hashing the declared key is re-keyed in place, one hashing a
39+
different key is re-generated, and a non-generated column of that name is
40+
refused rather than dropped.
41+
42+
Deliberately a real key comparison and not a blanket skip of every shadow. A
43+
shadow written before #12998 hashes the RAW columns, so `CONCAT` yields NULL for
44+
every NULL-organization row and the rows the `COALESCE(organization_id,
45+
'__global__')` bucket exists to constrain are constrained by nothing (#5030's
46+
shape) — indistinguishable by name from a healthy shadow. Skipping shadows
47+
wholesale would have traded one false destructive finding for a true silent one;
48+
that case is now reported as the ADR-0120 D4 tightening it is, runs the
49+
duplicate pre-flight before anything is dropped, and is repaired by the apply.
50+
A carrier whose expression cannot be read at all reports nothing rather than
51+
proposing a drop it cannot reason about.

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

Lines changed: 165 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,78 @@ export function isHashShadowColumn(name: string): boolean {
389389
return name.endsWith(HASH_SHADOW_SUFFIX);
390390
}
391391

392+
/**
393+
* The hash-shadow column that carries `indexName` (#11627), capped to MySQL's
394+
* 64-character identifier limit.
395+
*
396+
* ⚠️ Lives HERE, beside {@link isHashShadowColumn}, rather than in the driver:
397+
* #13015 was the price of the split. The ORPHAN-column pass knew the shadow
398+
* vocabulary and the INDEX differ did not, so a healthy shadow-carried UNIQUE
399+
* had its column protected from a drop while the index that column carries was
400+
* proposed for a destructive rebuild. Both passes now ask the same module the
401+
* same question, and `SqlDriver.hashShadowColumnFor` delegates here so the name
402+
* the sync CREATES and the name the differ LOOKS FOR cannot drift apart.
403+
*
404+
* Derived from the INDEX name rather than from the column list, deliberately:
405+
* one shadow serves one declared index, a composite index has no single column
406+
* to name it after, and the index name is already the differ's identity for the
407+
* constraint. The overflow branch keeps a truncated prefix for readability and
408+
* appends a digest of the FULL name, so two long index names that share a
409+
* prefix still get different shadows.
410+
*/
411+
export function hashShadowColumnFor(indexName: string): string {
412+
const direct = `${indexName}${HASH_SHADOW_SUFFIX}`;
413+
if (direct.length <= 64) return direct;
414+
const digest = createHash('sha256').update(indexName).digest('hex').slice(0, 8);
415+
const keep = 64 - HASH_SHADOW_SUFFIX.length - digest.length - 1;
416+
return `${indexName.slice(0, keep)}_${digest}${HASH_SHADOW_SUFFIX}`;
417+
}
418+
419+
/**
420+
* One key part a hash shadow hashes: the column identity, and whether the
421+
* generation expression folds it through the NULL-safe `COALESCE(col, ...)`
422+
* form (ADR-0120 D3, carried into the shadow by #12998).
423+
*/
424+
export interface HashShadowKeyPart {
425+
column: string;
426+
nullSafe: boolean;
427+
}
428+
429+
/**
430+
* Read the DECLARED key parts back out of a hash shadow's stored
431+
* `GENERATION_EXPRESSION` (#13015).
432+
*
433+
* This is what makes a shadow-carried key COMPARABLE rather than merely
434+
* skippable. Since #12998 the expression carries the NULL-safe parts in their
435+
* COALESCE spelling, so the FORM of the key — which columns, and which of them
436+
* are folded — survives the round trip, and the differ can ask the real
437+
* question ("does this shadow enforce what metadata declares?") instead of the
438+
* blind one ("is this a shadow at all?").
439+
*
440+
* ⛔ Why the blind question is not good enough: a shadow created BEFORE #12998
441+
* hashes the RAW columns, so `CONCAT` returns NULL for every NULL-organization
442+
* row and the rows the COALESCE bucket exists to constrain are constrained by
443+
* nothing (#5030's shape). It is indistinguishable BY NAME from a healthy one.
444+
* Skipping every shadow would make that class of drift permanently invisible —
445+
* trading a false destructive finding for a true silent one.
446+
*
447+
* MySQL stores the expression normalized and back-quoted — e.g.
448+
* `unhex(sha2(concat(coalesce(`org`,_utf8mb4'__global__'),0x1f,`v`),256))` — so
449+
* the key parts are exactly the back-quoted runs in key order, and the optional
450+
* `coalesce(` prefix marks the folded ones. The `unhex` / `sha2` / `concat`
451+
* wrapper and the `_utf8mb4'...'` literal carry no back quotes and contribute
452+
* nothing.
453+
*/
454+
export function parseHashShadowKeyParts(generationExpression: string): HashShadowKeyPart[] {
455+
const matches = String(generationExpression ?? '').matchAll(
456+
/(coalesce\s*\(\s*)?`((?:[^`]|``)+)`/gi,
457+
);
458+
return [...matches].map((m) => ({
459+
column: m[2]!.replace(/``/g, '`'),
460+
nullSafe: m[1] != null,
461+
}));
462+
}
463+
392464
/** Minimal shape of an introspected physical column (see SqlDriver.introspectColumns). */
393465
export interface PhysicalColumn {
394466
name: string;
@@ -1416,6 +1488,17 @@ export interface PhysicalIndex {
14161488
* key part is a plain column.
14171489
*/
14181490
nullSafeColumns?: string[];
1491+
/**
1492+
* When this index is physically carried by a #11627 hash shadow, the
1493+
* DECLARED key parts that shadow hashes, read back from the generation
1494+
* expression (#13015 via #12998) by `SqlDriver.introspectIndexes`.
1495+
*
1496+
* Absent both when the index is NOT shadow-carried and when it is but the
1497+
* expression could not be read. {@link isHashShadowCarrier} tells those two
1498+
* apart — the differ compares the resolved key, and treats the unresolved
1499+
* carrier as not-ours-to-reconcile rather than as drift.
1500+
*/
1501+
shadowKey?: HashShadowKeyPart[];
14191502
}
14201503

14211504
/**
@@ -1851,7 +1934,44 @@ function indexSignature(
18511934
* any `COALESCE(col, <literal>)` folds NULL into one bucket, so two spellings
18521935
* of the literal are the same constraint and must not read as drift.
18531936
*/
1854-
function canonicalIndexKey(columns: string[], nullSafeColumns?: ReadonlyArray<string> | null): string {
1937+
/**
1938+
* Is this physical index carried by a #11627 hash shadow — i.e. does it key
1939+
* exactly the one driver-owned generated column that stands in for the
1940+
* declared key MySQL could not express directly?
1941+
*
1942+
* Answerable from the index alone, by NAME: the shadow is derived from the
1943+
* index name ({@link hashShadowColumnFor}), so a carrier is an index whose sole
1944+
* key column is its own shadow. That is what makes this the FAIL-SAFE half of
1945+
* #13015 — it holds even when the generation expression cannot be read, and a
1946+
* carrier is never a thing this differ may propose destroying on a guess.
1947+
*/
1948+
export function isHashShadowCarrier(index: PhysicalIndex): boolean {
1949+
return index.columns.length === 1 && index.columns[0] === hashShadowColumnFor(index.name);
1950+
}
1951+
1952+
/**
1953+
* The key an index ENFORCES, which is not always the key it STORES (#13015).
1954+
*
1955+
* For an ordinary index the two are the same. For a #11627 shadow-carried
1956+
* UNIQUE the stored key is one VARBINARY(32) generated column and the enforced
1957+
* key is the declared column set the shadow hashes — so every comparison in
1958+
* this module has to run against THIS, or a healthy constraint reads as an
1959+
* index over a column no metadata declares.
1960+
*/
1961+
export function enforcedIndexKey(index: PhysicalIndex): {
1962+
columns: string[];
1963+
nullSafeColumns?: string[];
1964+
} {
1965+
if (!index.shadowKey) {
1966+
return { columns: index.columns, nullSafeColumns: index.nullSafeColumns };
1967+
}
1968+
return {
1969+
columns: index.shadowKey.map((k) => k.column),
1970+
nullSafeColumns: index.shadowKey.filter((k) => k.nullSafe).map((k) => k.column),
1971+
};
1972+
}
1973+
1974+
export function canonicalIndexKey(columns: string[], nullSafeColumns?: ReadonlyArray<string> | null): string {
18551975
const ns = new Set(nullSafeColumns ?? []);
18561976
return columns.map((c) => (ns.has(c) ? `coalesce:${c}` : c)).join(',');
18571977
}
@@ -1905,6 +2025,11 @@ export function diffManagedIndexes(args: {
19052025
if (!p || p.primary || isRuntimeManagedIndex(p, runtimeCreated, tenantField)) return false;
19062026
if (!p.unique || p.partial === true) return false;
19072027
if ((p.expressions?.length ?? 0) > 0 || (p.nullSafeColumns?.length ?? 0) > 0) return false;
2028+
// #13015: nor is a hash-shadow carrier. Its stored key is one generated
2029+
// column, so the identity comparison below already excludes it — stated
2030+
// outright because the exclusion must survive that comparison changing,
2031+
// and because `replace_unique_index` DROPS the legacy name.
2032+
if (isHashShadowCarrier(p)) return false;
19082033
return (
19092034
p.columns.length === l.legacyColumns.length &&
19102035
p.columns.every((c, i) => c === l.legacyColumns[i])
@@ -1968,10 +2093,13 @@ export function diffManagedIndexes(args: {
19682093
continue;
19692094
}
19702095
// Same normalization on BOTH sides (#4884, ADR-0120 D3): column identity
1971-
// AND key-part form, literal-agnostic on the COALESCE literal.
2096+
// AND key-part form, literal-agnostic on the COALESCE literal — asked of
2097+
// the key the index ENFORCES, which for a #11627 shadow-carried UNIQUE is
2098+
// not the column it stores (#13015).
2099+
const pk = enforcedIndexKey(p);
19722100
if (
19732101
p.unique === e.unique &&
1974-
canonicalIndexKey(p.columns, p.nullSafeColumns) === canonicalIndexKey(e.columns, e.nullSafeColumns)
2102+
canonicalIndexKey(pk.columns, pk.nullSafeColumns) === canonicalIndexKey(e.columns, e.nullSafeColumns)
19752103
) {
19762104
continue;
19772105
}
@@ -1984,6 +2112,20 @@ export function diffManagedIndexes(args: {
19842112
// (`recreate_index` → drop first) this differ cannot undo. Not ours to
19852113
// reconcile (#4884).
19862114
if (isRuntimeManagedIndex(p, runtimeCreated, tenantField)) continue;
2115+
// #13015, fail-safe half: a hash-shadow carrier whose generation
2116+
// expression could NOT be read (`shadowKey` unresolved). We know by name
2117+
// that the index is driver-owned and that its stored key is a digest, so
2118+
// the identity comparison above is meaningless for it — but we do not know
2119+
// WHAT it hashes, and the remedy below is a DROP. Report nothing rather
2120+
// than propose destroying a constraint on a guess.
2121+
//
2122+
// ⛔ The `!p.shadowKey` half is load-bearing, and was measured: without it
2123+
// this guard swallows the RESOLVED carriers too, which silently demotes the
2124+
// whole fix to the blind skip — every shadow-carried index unreportable,
2125+
// including a pre-#12998 one hashing the RAW columns whose constraint does
2126+
// not cover NULL-organization rows at all. Green, quiet, and the exact
2127+
// trade this fix exists to refuse.
2128+
if (isHashShadowCarrier(p) && !p.shadowKey) continue;
19872129
// Same name, different definition. `syncDeclaredIndexes` skips by name, so
19882130
// this never self-heals: it has to be dropped and rebuilt. Tightening to
19892131
// UNIQUE is destructive — the CREATE can fail on existing duplicates, and
@@ -1995,20 +2137,30 @@ export function diffManagedIndexes(args: {
19952137
// marked so the driver can run the duplicate pre-flight probe on it:
19962138
// clean → recategorised `safe` (dev autoMigrate may apply); duplicates →
19972139
// blocked with a row report, the old index left in place.
2140+
//
2141+
// #13015: read through the ENFORCED key, so a pre-#12998 shadow — same
2142+
// columns, hashed RAW instead of through the NULL-safe COALESCE — is
2143+
// recognised as exactly this tightening and gets the same duplicate
2144+
// pre-flight before anything is dropped. The explicit "physical side is
2145+
// bare" clause is what `p.expressions.length === 0` used to imply on its
2146+
// own (`nullSafeColumns` is only ever recorded alongside an expression key
2147+
// part); a resolved shadow key can carry NULL-safe parts with no
2148+
// expressions at all, so the implication no longer holds.
19982149
const tightenNullSafeOnly =
19992150
e.unique &&
20002151
p.unique &&
20012152
(e.nullSafeColumns?.length ?? 0) > 0 &&
20022153
(p.expressions?.length ?? 0) === 0 &&
2154+
(pk.nullSafeColumns?.length ?? 0) === 0 &&
20032155
p.partial !== true &&
2004-
p.columns.join(',') === e.columns.join(',');
2156+
pk.columns.join(',') === e.columns.join(',');
20052157
out.push({
20062158
kind: 'index_mismatch',
20072159
remoteName: table,
20082160
table,
20092161
column: e.columns[0],
20102162
expected: indexSignature(e.columns, e.unique, e.nullSafeColumns),
2011-
actual: indexSignature(p.columns, p.unique, p.nullSafeColumns),
2163+
actual: indexSignature(pk.columns, p.unique, pk.nullSafeColumns),
20122164
severity: e.unique ? 'error' : 'warning',
20132165
category: e.unique ? 'destructive' : 'needs_confirm',
20142166
op: {
@@ -2022,11 +2174,11 @@ export function diffManagedIndexes(args: {
20222174
...(tightenNullSafeOnly ? { tightenNullSafeOnly: true } : {}),
20232175
},
20242176
message: tightenNullSafeOnly
2025-
? `${table}: index '${e.name}' is ${indexSignature(p.columns, p.unique, p.nullSafeColumns)} but metadata declares ` +
2177+
? `${table}: index '${e.name}' is ${indexSignature(pk.columns, p.unique, pk.nullSafeColumns)} but metadata declares ` +
20262178
`${indexSignature(e.columns, e.unique, e.nullSafeColumns)} (ADR-0120 D3: the organization key part is NULL-safe, ` +
20272179
`so rows without an organization are constrained too). Pure tightening — eligibility is decided by the ` +
20282180
`duplicate pre-flight probe.`
2029-
: `${table}: index '${e.name}' is ${indexSignature(p.columns, p.unique, p.nullSafeColumns)} but metadata declares ` +
2181+
: `${table}: index '${e.name}' is ${indexSignature(pk.columns, p.unique, pk.nullSafeColumns)} but metadata declares ` +
20302182
`${indexSignature(e.columns, e.unique, e.nullSafeColumns)} — the additive sync skips it by name, so it must be rebuilt` +
20312183
(e.unique
20322184
? `. Creating the UNIQUE index can fail on existing duplicates: "os migrate apply --allow-destructive".`
@@ -2045,18 +2197,22 @@ export function diffManagedIndexes(args: {
20452197
// (#4884 — the boot advised dropping `idx_sys_metadata_overlay_draft`, the
20462198
// partial UNIQUE enforcing draft-overlay uniqueness, on a healthy fresh DB).
20472199
if (isRuntimeManagedIndex(p, runtimeCreated, tenantField)) continue;
2200+
// #13015: an orphaned shadow carrier is still an orphan — its declaration
2201+
// is gone, and `drop_index` is the right remedy — but the report must name
2202+
// the constraint it enforced, not the digest column it stored.
2203+
const po = enforcedIndexKey(p);
20482204
out.push({
20492205
kind: 'unmapped_index',
20502206
remoteName: table,
20512207
table,
20522208
column: p.columns[0],
20532209
expected: '(absent)',
2054-
actual: indexSignature(p.columns, p.unique, p.nullSafeColumns),
2210+
actual: indexSignature(po.columns, p.unique, po.nullSafeColumns),
20552211
severity: 'warning',
20562212
category: 'destructive',
20572213
op: { type: 'drop_index', table, column: p.columns[0], indexName: p.name },
20582214
message:
2059-
`${table}: index '${p.name}' ${indexSignature(p.columns, p.unique, p.nullSafeColumns)} carries ObjectStack's generated naming ` +
2215+
`${table}: index '${p.name}' ${indexSignature(po.columns, p.unique, po.nullSafeColumns)} carries ObjectStack's generated naming ` +
20602216
`but matches no declared index (orphaned) — "os migrate apply --allow-destructive" to drop it.`,
20612217
});
20622218
}

0 commit comments

Comments
 (0)