From 2b1593e104579bc4c0dbeb826796bcb2ad8863d5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 01:02:02 +0000 Subject: [PATCH] fix(driver-sql): a shadow-carried UNIQUE is not index drift, and its remedy dropped the constraint 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. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry --- ...hadow-carried-unique-is-not-index-drift.md | 51 +++ .../drivers/driver-sql/src/schema-drift.ts | 174 +++++++- ...r-13015-shadow-carried-index-drift.test.ts | 381 ++++++++++++++++++ packages/drivers/driver-sql/src/sql-driver.ts | 200 ++++++++- 4 files changed, 778 insertions(+), 28 deletions(-) create mode 100644 .changeset/shadow-carried-unique-is-not-index-drift.md create mode 100644 packages/drivers/driver-sql/src/sql-driver-13015-shadow-carried-index-drift.test.ts diff --git a/.changeset/shadow-carried-unique-is-not-index-drift.md b/.changeset/shadow-carried-unique-is-not-index-drift.md new file mode 100644 index 0000000000..0e48aa2f2c --- /dev/null +++ b/.changeset/shadow-carried-unique-is-not-index-drift.md @@ -0,0 +1,51 @@ +--- +'@objectstack/driver-sql': patch +--- + +A healthy hash-shadow-carried UNIQUE is no longer reported as destructive index +drift — and the remedy that used to be proposed for it would have DROPPED the +constraint + +On MySQL a declared UNIQUE whose key is too wide for an InnoDB key part is +carried by a driver-owned generated column holding a SHA-256 of the key values +(#11627). The index differ compared the declared columns against the columns an +index physically KEYS, so a shadow-carried UNIQUE — one VARBINARY(32) generated +column as the whole key — could never match. A clean `initObjects` reported the +index the same boot had just created as `index_mismatch` / `destructive`, with +`recreate_index` as the remedy and `os migrate apply --allow-destructive` in the +message. + +Following that advice removed a live uniqueness guarantee. `recreate_index` +drops the UNIQUE by name and re-runs the additive sync; the sync retakes the +shadow route, and its `ALTER TABLE … ADD COLUMN` then failed on the generated +column that **survived** the index drop. That failure is a duplicate-COLUMN +error, matched by neither the "already exists" absorb (which spells index names) +nor the unique-violation branch — so the apply ended with the constraint dropped +and not re-created. + +Both halves are fixed, and they share one vocabulary rather than special-casing +the differ. The orphan-COLUMN pass already recognised the shadow as driver-owned +(`isHashShadowColumn`) while the index it carries was proposed for destructive +rebuild; that asymmetry was the shape of the defect. + +- The shadow's name derivation moved next to that predicate, so the name the + sync creates and the name the differ looks for have one definition. +- Introspection reads the shadow's stored `GENERATION_EXPRESSION` and records + the key it actually hashes, so the differ compares the key the constraint + **enforces** instead of the digest column it stores. Drift reports and plan + messages now name that key too, rather than `UNIQUE (uniq_…__hash)`. +- The sync inspects a surviving shadow column instead of assuming it absent: a + column already hashing the declared key is re-keyed in place, one hashing a + different key is re-generated, and a non-generated column of that name is + refused rather than dropped. + +Deliberately a real key comparison and not a blanket skip of every shadow. A +shadow written before #12998 hashes the RAW columns, so `CONCAT` yields NULL for +every NULL-organization row and the rows the `COALESCE(organization_id, +'__global__')` bucket exists to constrain are constrained by nothing (#5030's +shape) — indistinguishable by name from a healthy shadow. Skipping shadows +wholesale would have traded one false destructive finding for a true silent one; +that case is now reported as the ADR-0120 D4 tightening it is, runs the +duplicate pre-flight before anything is dropped, and is repaired by the apply. +A carrier whose expression cannot be read at all reports nothing rather than +proposing a drop it cannot reason about. diff --git a/packages/drivers/driver-sql/src/schema-drift.ts b/packages/drivers/driver-sql/src/schema-drift.ts index 20db330554..39f7154227 100644 --- a/packages/drivers/driver-sql/src/schema-drift.ts +++ b/packages/drivers/driver-sql/src/schema-drift.ts @@ -389,6 +389,78 @@ export function isHashShadowColumn(name: string): boolean { return name.endsWith(HASH_SHADOW_SUFFIX); } +/** + * The hash-shadow column that carries `indexName` (#11627), capped to MySQL's + * 64-character identifier limit. + * + * ⚠️ Lives HERE, beside {@link isHashShadowColumn}, rather than in the driver: + * #13015 was the price of the split. The ORPHAN-column pass knew the shadow + * vocabulary and the INDEX differ did not, so a healthy shadow-carried UNIQUE + * had its column protected from a drop while the index that column carries was + * proposed for a destructive rebuild. Both passes now ask the same module the + * same question, and `SqlDriver.hashShadowColumnFor` delegates here so the name + * the sync CREATES and the name the differ LOOKS FOR cannot drift apart. + * + * Derived from the INDEX name rather than from the column list, deliberately: + * one shadow serves one declared index, a composite index has no single column + * to name it after, and the index name is already the differ's identity for the + * constraint. The overflow branch keeps a truncated prefix for readability and + * appends a digest of the FULL name, so two long index names that share a + * prefix still get different shadows. + */ +export function hashShadowColumnFor(indexName: string): string { + const direct = `${indexName}${HASH_SHADOW_SUFFIX}`; + if (direct.length <= 64) return direct; + const digest = createHash('sha256').update(indexName).digest('hex').slice(0, 8); + const keep = 64 - HASH_SHADOW_SUFFIX.length - digest.length - 1; + return `${indexName.slice(0, keep)}_${digest}${HASH_SHADOW_SUFFIX}`; +} + +/** + * One key part a hash shadow hashes: the column identity, and whether the + * generation expression folds it through the NULL-safe `COALESCE(col, ...)` + * form (ADR-0120 D3, carried into the shadow by #12998). + */ +export interface HashShadowKeyPart { + column: string; + nullSafe: boolean; +} + +/** + * Read the DECLARED key parts back out of a hash shadow's stored + * `GENERATION_EXPRESSION` (#13015). + * + * This is what makes a shadow-carried key COMPARABLE rather than merely + * skippable. Since #12998 the expression carries the NULL-safe parts in their + * COALESCE spelling, so the FORM of the key — which columns, and which of them + * are folded — survives the round trip, and the differ can ask the real + * question ("does this shadow enforce what metadata declares?") instead of the + * blind one ("is this a shadow at all?"). + * + * ⛔ Why the blind question is not good enough: a shadow created BEFORE #12998 + * hashes the RAW columns, so `CONCAT` returns NULL for every NULL-organization + * row and the rows the COALESCE bucket exists to constrain are constrained by + * nothing (#5030's shape). It is indistinguishable BY NAME from a healthy one. + * Skipping every shadow would make that class of drift permanently invisible — + * trading a false destructive finding for a true silent one. + * + * MySQL stores the expression normalized and back-quoted — e.g. + * `unhex(sha2(concat(coalesce(`org`,_utf8mb4'__global__'),0x1f,`v`),256))` — so + * the key parts are exactly the back-quoted runs in key order, and the optional + * `coalesce(` prefix marks the folded ones. The `unhex` / `sha2` / `concat` + * wrapper and the `_utf8mb4'...'` literal carry no back quotes and contribute + * nothing. + */ +export function parseHashShadowKeyParts(generationExpression: string): HashShadowKeyPart[] { + const matches = String(generationExpression ?? '').matchAll( + /(coalesce\s*\(\s*)?`((?:[^`]|``)+)`/gi, + ); + return [...matches].map((m) => ({ + column: m[2]!.replace(/``/g, '`'), + nullSafe: m[1] != null, + })); +} + /** Minimal shape of an introspected physical column (see SqlDriver.introspectColumns). */ export interface PhysicalColumn { name: string; @@ -1416,6 +1488,17 @@ export interface PhysicalIndex { * key part is a plain column. */ nullSafeColumns?: string[]; + /** + * When this index is physically carried by a #11627 hash shadow, the + * DECLARED key parts that shadow hashes, read back from the generation + * expression (#13015 via #12998) by `SqlDriver.introspectIndexes`. + * + * Absent both when the index is NOT shadow-carried and when it is but the + * expression could not be read. {@link isHashShadowCarrier} tells those two + * apart — the differ compares the resolved key, and treats the unresolved + * carrier as not-ours-to-reconcile rather than as drift. + */ + shadowKey?: HashShadowKeyPart[]; } /** @@ -1851,7 +1934,44 @@ function indexSignature( * any `COALESCE(col, )` folds NULL into one bucket, so two spellings * of the literal are the same constraint and must not read as drift. */ -function canonicalIndexKey(columns: string[], nullSafeColumns?: ReadonlyArray | null): string { +/** + * Is this physical index carried by a #11627 hash shadow — i.e. does it key + * exactly the one driver-owned generated column that stands in for the + * declared key MySQL could not express directly? + * + * Answerable from the index alone, by NAME: the shadow is derived from the + * index name ({@link hashShadowColumnFor}), so a carrier is an index whose sole + * key column is its own shadow. That is what makes this the FAIL-SAFE half of + * #13015 — it holds even when the generation expression cannot be read, and a + * carrier is never a thing this differ may propose destroying on a guess. + */ +export function isHashShadowCarrier(index: PhysicalIndex): boolean { + return index.columns.length === 1 && index.columns[0] === hashShadowColumnFor(index.name); +} + +/** + * The key an index ENFORCES, which is not always the key it STORES (#13015). + * + * For an ordinary index the two are the same. For a #11627 shadow-carried + * UNIQUE the stored key is one VARBINARY(32) generated column and the enforced + * key is the declared column set the shadow hashes — so every comparison in + * this module has to run against THIS, or a healthy constraint reads as an + * index over a column no metadata declares. + */ +export function enforcedIndexKey(index: PhysicalIndex): { + columns: string[]; + nullSafeColumns?: string[]; +} { + if (!index.shadowKey) { + return { columns: index.columns, nullSafeColumns: index.nullSafeColumns }; + } + return { + columns: index.shadowKey.map((k) => k.column), + nullSafeColumns: index.shadowKey.filter((k) => k.nullSafe).map((k) => k.column), + }; +} + +export function canonicalIndexKey(columns: string[], nullSafeColumns?: ReadonlyArray | null): string { const ns = new Set(nullSafeColumns ?? []); return columns.map((c) => (ns.has(c) ? `coalesce:${c}` : c)).join(','); } @@ -1905,6 +2025,11 @@ export function diffManagedIndexes(args: { if (!p || p.primary || isRuntimeManagedIndex(p, runtimeCreated, tenantField)) return false; if (!p.unique || p.partial === true) return false; if ((p.expressions?.length ?? 0) > 0 || (p.nullSafeColumns?.length ?? 0) > 0) return false; + // #13015: nor is a hash-shadow carrier. Its stored key is one generated + // column, so the identity comparison below already excludes it — stated + // outright because the exclusion must survive that comparison changing, + // and because `replace_unique_index` DROPS the legacy name. + if (isHashShadowCarrier(p)) return false; return ( p.columns.length === l.legacyColumns.length && p.columns.every((c, i) => c === l.legacyColumns[i]) @@ -1968,10 +2093,13 @@ export function diffManagedIndexes(args: { continue; } // Same normalization on BOTH sides (#4884, ADR-0120 D3): column identity - // AND key-part form, literal-agnostic on the COALESCE literal. + // AND key-part form, literal-agnostic on the COALESCE literal — asked of + // the key the index ENFORCES, which for a #11627 shadow-carried UNIQUE is + // not the column it stores (#13015). + const pk = enforcedIndexKey(p); if ( p.unique === e.unique && - canonicalIndexKey(p.columns, p.nullSafeColumns) === canonicalIndexKey(e.columns, e.nullSafeColumns) + canonicalIndexKey(pk.columns, pk.nullSafeColumns) === canonicalIndexKey(e.columns, e.nullSafeColumns) ) { continue; } @@ -1984,6 +2112,20 @@ export function diffManagedIndexes(args: { // (`recreate_index` → drop first) this differ cannot undo. Not ours to // reconcile (#4884). if (isRuntimeManagedIndex(p, runtimeCreated, tenantField)) continue; + // #13015, fail-safe half: a hash-shadow carrier whose generation + // expression could NOT be read (`shadowKey` unresolved). We know by name + // that the index is driver-owned and that its stored key is a digest, so + // the identity comparison above is meaningless for it — but we do not know + // WHAT it hashes, and the remedy below is a DROP. Report nothing rather + // than propose destroying a constraint on a guess. + // + // ⛔ The `!p.shadowKey` half is load-bearing, and was measured: without it + // this guard swallows the RESOLVED carriers too, which silently demotes the + // whole fix to the blind skip — every shadow-carried index unreportable, + // including a pre-#12998 one hashing the RAW columns whose constraint does + // not cover NULL-organization rows at all. Green, quiet, and the exact + // trade this fix exists to refuse. + if (isHashShadowCarrier(p) && !p.shadowKey) continue; // Same name, different definition. `syncDeclaredIndexes` skips by name, so // this never self-heals: it has to be dropped and rebuilt. Tightening to // UNIQUE is destructive — the CREATE can fail on existing duplicates, and @@ -1995,20 +2137,30 @@ export function diffManagedIndexes(args: { // marked so the driver can run the duplicate pre-flight probe on it: // clean → recategorised `safe` (dev autoMigrate may apply); duplicates → // blocked with a row report, the old index left in place. + // + // #13015: read through the ENFORCED key, so a pre-#12998 shadow — same + // columns, hashed RAW instead of through the NULL-safe COALESCE — is + // recognised as exactly this tightening and gets the same duplicate + // pre-flight before anything is dropped. The explicit "physical side is + // bare" clause is what `p.expressions.length === 0` used to imply on its + // own (`nullSafeColumns` is only ever recorded alongside an expression key + // part); a resolved shadow key can carry NULL-safe parts with no + // expressions at all, so the implication no longer holds. const tightenNullSafeOnly = e.unique && p.unique && (e.nullSafeColumns?.length ?? 0) > 0 && (p.expressions?.length ?? 0) === 0 && + (pk.nullSafeColumns?.length ?? 0) === 0 && p.partial !== true && - p.columns.join(',') === e.columns.join(','); + pk.columns.join(',') === e.columns.join(','); out.push({ kind: 'index_mismatch', remoteName: table, table, column: e.columns[0], expected: indexSignature(e.columns, e.unique, e.nullSafeColumns), - actual: indexSignature(p.columns, p.unique, p.nullSafeColumns), + actual: indexSignature(pk.columns, p.unique, pk.nullSafeColumns), severity: e.unique ? 'error' : 'warning', category: e.unique ? 'destructive' : 'needs_confirm', op: { @@ -2022,11 +2174,11 @@ export function diffManagedIndexes(args: { ...(tightenNullSafeOnly ? { tightenNullSafeOnly: true } : {}), }, message: tightenNullSafeOnly - ? `${table}: index '${e.name}' is ${indexSignature(p.columns, p.unique, p.nullSafeColumns)} but metadata declares ` + + ? `${table}: index '${e.name}' is ${indexSignature(pk.columns, p.unique, pk.nullSafeColumns)} but metadata declares ` + `${indexSignature(e.columns, e.unique, e.nullSafeColumns)} (ADR-0120 D3: the organization key part is NULL-safe, ` + `so rows without an organization are constrained too). Pure tightening — eligibility is decided by the ` + `duplicate pre-flight probe.` - : `${table}: index '${e.name}' is ${indexSignature(p.columns, p.unique, p.nullSafeColumns)} but metadata declares ` + + : `${table}: index '${e.name}' is ${indexSignature(pk.columns, p.unique, pk.nullSafeColumns)} but metadata declares ` + `${indexSignature(e.columns, e.unique, e.nullSafeColumns)} — the additive sync skips it by name, so it must be rebuilt` + (e.unique ? `. Creating the UNIQUE index can fail on existing duplicates: "os migrate apply --allow-destructive".` @@ -2045,18 +2197,22 @@ export function diffManagedIndexes(args: { // (#4884 — the boot advised dropping `idx_sys_metadata_overlay_draft`, the // partial UNIQUE enforcing draft-overlay uniqueness, on a healthy fresh DB). if (isRuntimeManagedIndex(p, runtimeCreated, tenantField)) continue; + // #13015: an orphaned shadow carrier is still an orphan — its declaration + // is gone, and `drop_index` is the right remedy — but the report must name + // the constraint it enforced, not the digest column it stored. + const po = enforcedIndexKey(p); out.push({ kind: 'unmapped_index', remoteName: table, table, column: p.columns[0], expected: '(absent)', - actual: indexSignature(p.columns, p.unique, p.nullSafeColumns), + actual: indexSignature(po.columns, p.unique, po.nullSafeColumns), severity: 'warning', category: 'destructive', op: { type: 'drop_index', table, column: p.columns[0], indexName: p.name }, message: - `${table}: index '${p.name}' ${indexSignature(p.columns, p.unique, p.nullSafeColumns)} carries ObjectStack's generated naming ` + + `${table}: index '${p.name}' ${indexSignature(po.columns, p.unique, po.nullSafeColumns)} carries ObjectStack's generated naming ` + `but matches no declared index (orphaned) — "os migrate apply --allow-destructive" to drop it.`, }); } diff --git a/packages/drivers/driver-sql/src/sql-driver-13015-shadow-carried-index-drift.test.ts b/packages/drivers/driver-sql/src/sql-driver-13015-shadow-carried-index-drift.test.ts new file mode 100644 index 0000000000..3c994f79d5 --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-13015-shadow-carried-index-drift.test.ts @@ -0,0 +1,381 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #13015 — a healthy #11627 hash-shadow UNIQUE is not drift, and the remedy the + * differ used to propose would have DROPPED the constraint it was reconciling. + * + * ## The defect + * + * `diffManagedIndexes` compared the declared key against the columns the index + * physically KEYS. A shadow-carried UNIQUE keys exactly one driver-owned + * VARBINARY(32) generated column, so that comparison could never match: a clean + * `initObjects` reported the index the same boot had just created as + * `index_mismatch` / `destructive` / `recreate_index`. + * + * ## Why the remedy was worse than the defect + * + * `recreate_index` drops the UNIQUE by name and re-runs the sync. The sync + * retakes the shadow route — and the shadow `ALTER TABLE ... ADD COLUMN` then + * failed on the SURVIVING generated column (dropping an index does not drop the + * column it keys). That failure is matched by neither the "already exists" + * absorb (which spells INDEX names) nor the unique-violation branch, so the + * apply ended with the constraint dropped and not re-created. An operator + * following `os migrate apply --allow-destructive`, as the finding's own + * message instructed, removed a live uniqueness guarantee. + * + * ## What is pinned here + * + * Half the vocabulary was already taught: the ORPHAN-column pass guards the + * shadow via `isHashShadowColumn` while the index it carries was proposed for + * destructive rebuild. The fix makes both passes read one vocabulary — so the + * pins below are as much about the differ NOT going quiet as about it going + * quiet in the right place. Every "no finding" assertion carries a COLOCATED + * positive control in the same `diffManagedIndexes` call: a genuinely drifted + * index that must still be reported. A fix that simply skipped every shadow + * would pass the first half and fail the stale-shadow pins. + * + * The live-MySQL cell reads the PHYSICAL catalog (`information_schema`) rather + * than the differ's own report about itself, and runs opt-in: + * + * OS_TEST_MYSQL_URL=mysql://root:root@127.0.0.1:3306/conformance \ + * pnpm --filter @objectstack/driver-sql test + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { SqlDriver, diffManagedIndexes } from '../src/index.js'; +// The shadow vocabulary stays INTERNAL to this package — nothing outside it +// consumes a hash shadow, so #13015 deliberately did not widen the published +// surface. Imported from the module, exactly as the #11627/#12998 suites do. +import { + enforcedIndexKey, + hashShadowColumnFor, + isHashShadowCarrier, + isHashShadowColumn, + parseHashShadowKeyParts, + type ExpectedIndex, + type PhysicalIndex, +} from './schema-drift.js'; +import { MYSQL_CELL, declareDialectCell } from './live-dialect-matrix.testkit.js'; + +// ───────────────────────────────────────────────────────────────────────── +// Fixtures — the shapes `introspectIndexes` produces on MySQL +// ───────────────────────────────────────────────────────────────────────── + +const TABLE = 'os13015_probe'; +/** The org-scoped declared unique whose key MySQL cannot express directly. */ +const DECLARED_NAME = 'uniq_os13015_probe_organization_id_v'; +const SHADOW = hashShadowColumnFor(DECLARED_NAME); + +const declaredOrgUnique: ExpectedIndex = { + name: DECLARED_NAME, + columns: ['organization_id', 'v'], + unique: true, + nullSafeColumns: ['organization_id'], +}; + +/** + * A shadow-carried index as the catalog reports it: ONE plain generated column + * as the whole key. `expressions` is empty — the shadow is a real column, not + * an EXPRESSION key part, which is precisely why `isRuntimeManagedIndex` never + * covered this case. + */ +const carrier = (shadowKey?: PhysicalIndex['shadowKey']): PhysicalIndex => ({ + name: DECLARED_NAME, + columns: [SHADOW], + unique: true, + ...(shadowKey ? { shadowKey } : {}), +}); + +/** What a HEALTHY (post-#12998) shadow hashes: the declared NULL-safe key. */ +const healthyKey = [ + { column: 'organization_id', nullSafe: true }, + { column: 'v', nullSafe: false }, +]; + +/** What a PRE-#12998 shadow hashes: the same columns, RAW. */ +const staleKey = [ + { column: 'organization_id', nullSafe: false }, + { column: 'v', nullSafe: false }, +]; + +/** + * The colocated POSITIVE CONTROL, present in every call below: an ordinary + * declared unique that really has drifted (declared NULL-safe, physically + * bare). It must keep producing a finding, so an empty result for the shadow + * can never be read as "the differ stopped reporting". + * + * Deliberately shares no key column and no name fragment with the shadow + * fixtures — the control must not be a substring of the term under test. + */ +const CONTROL_NAME = 'uniq_os13015_probe_organization_id_ctl'; +const declaredControl: ExpectedIndex = { + name: CONTROL_NAME, + columns: ['organization_id', 'ctl'], + unique: true, + nullSafeColumns: ['organization_id'], +}; +const physicalControl: PhysicalIndex = { + name: CONTROL_NAME, + columns: ['organization_id', 'ctl'], + unique: true, +}; + +const diff = (expected: ExpectedIndex[], physical: PhysicalIndex[]) => + diffManagedIndexes({ + table: TABLE, + expected, + legacy: [], + physical, + tenantField: 'organization_id', + }); + +describe('shadow-carried UNIQUE is not index drift (#13015)', () => { + it('derives the shadow column from the index name, and recognises the carrier', () => { + expect(SHADOW).toBe(`${DECLARED_NAME}__hash`); + expect(isHashShadowColumn(SHADOW)).toBe(true); + expect(isHashShadowCarrier(carrier())).toBe(true); + // Not a carrier: an ordinary index, and a shadow-named column keyed by an + // index it does NOT belong to (the name binds shadow to index, #11627). + expect(isHashShadowCarrier(physicalControl)).toBe(false); + expect(isHashShadowCarrier({ name: 'uniq_other', columns: [SHADOW], unique: true })).toBe(false); + }); + + it('reports NO drift for a healthy shadow-carried unique, while still reporting real drift', () => { + const entries = diff([declaredOrgUnique, declaredControl], [carrier(healthyKey), physicalControl]); + // The positive control fired — the differ is awake. + expect(entries.map((e) => e.op.type)).toEqual(['recreate_index']); + expect((entries[0]!.op as any).indexName).toBe(CONTROL_NAME); + // …and said nothing at all about the shadow-carried one. + expect(entries.filter((e) => (e.op as any).indexName === DECLARED_NAME)).toEqual([]); + }); + + it('reports no drift for an UNRESOLVED carrier rather than proposing a destructive drop', () => { + // The catalog read that resolves the generation expression can fail. The + // carrier is still recognisable by name, so the differ must decline to + // reason about it — never propose dropping a constraint on a guess. + const entries = diff([declaredOrgUnique, declaredControl], [carrier(), physicalControl]); + expect(entries.map((e) => (e.op as any).indexName)).toEqual([CONTROL_NAME]); + }); + + it('STILL reports a pre-#12998 shadow that hashes the RAW columns', () => { + // The case a blind skip would have made permanently invisible: same + // columns, no COALESCE, so every NULL-organization row is unconstrained + // (#5030's shape) while the boot log calls the constraint carried. + const entries = diff([declaredOrgUnique], [carrier(staleKey)]); + expect(entries.length).toBe(1); + const [entry] = entries; + expect(entry!.op.type).toBe('recreate_index'); + // Recognised as the ADR-0120 D4 pure tightening, so the duplicate + // pre-flight runs before anything is dropped. + expect((entry!.op as any).tightenNullSafeOnly).toBe(true); + // The report names the key the index ENFORCES, not the digest column it + // stores — the old message read `UNIQUE (uniq_..._v__hash)`, which told an + // operator nothing about the constraint at risk. + expect(entry!.actual).toBe('UNIQUE (organization_id, v)'); + expect(entry!.actual).not.toContain('__hash'); + expect(entry!.expected).toBe("UNIQUE (COALESCE(organization_id, '__global__'), v)"); + }); + + it('describes an ORPHANED carrier by the key it enforced, not by its digest column', () => { + // Declaration gone: `drop_index` is still the right remedy, but the report + // must be readable. + const entries = diff([], [carrier(healthyKey)]); + expect(entries.length).toBe(1); + expect(entries[0]!.op.type).toBe('drop_index'); + expect(entries[0]!.actual).toBe("UNIQUE (COALESCE(organization_id, '__global__'), v)"); + expect(entries[0]!.message).not.toContain('__hash'); + }); + + it('resolves the enforced key from the stored generation expression', () => { + // The spellings MySQL 8 stores, verbatim: single column, plain composite, + // and the NULL-safe composite #12998 introduced. + expect(parseHashShadowKeyParts('unhex(sha2(`v`,256))')).toEqual([ + { column: 'v', nullSafe: false }, + ]); + expect(parseHashShadowKeyParts('unhex(sha2(concat(`a`,0x1f,`b`),256))')).toEqual([ + { column: 'a', nullSafe: false }, + { column: 'b', nullSafe: false }, + ]); + expect( + parseHashShadowKeyParts( + "unhex(sha2(concat(coalesce(`organization_id`,_utf8mb4'__global__'),0x1f,`v`),256))", + ), + ).toEqual(healthyKey); + // An unreadable expression resolves to nothing — which is what keeps the + // carrier in the "declines to reason about it" branch above. + expect(parseHashShadowKeyParts('')).toEqual([]); + expect(enforcedIndexKey(carrier())).toEqual({ columns: [SHADOW], nullSafeColumns: undefined }); + expect(enforcedIndexKey(carrier(healthyKey))).toEqual({ + columns: ['organization_id', 'v'], + nullSafeColumns: ['organization_id'], + }); + }); +}); + +// ───────────────────────────────────────────────────────────────────────── +// Live MySQL: the PHYSICAL catalog, before and after the remedy runs +// ───────────────────────────────────────────────────────────────────────── + +/** + * An org-scoped unique over a field too long for a MySQL key part, so the sync + * is forced down the #11627 shadow route — the same route the live platform + * members take. + */ +const orgUniqueOn = (name: string) => ({ + name, + fields: { + organization_id: { type: 'string' }, + v: { type: 'text', maxLength: 1024 }, + }, + indexes: [{ fields: ['v'], unique: 'organization' as const, name: `uniq_${name}_org_v` }], +}); + +declareDialectCell(MYSQL_CELL, 'shadow-carried index drift (#13015)', (cell) => { + describe('shadow-carried UNIQUE against the live MySQL catalog (#13015)', () => { + let driver: SqlDriver; + afterEach(async () => { + await driver?.disconnect().catch(() => {}); + }); + + /** Physical truth, read from the catalog — never from the DDL we emitted. */ + const catalog = async (table: string) => { + const knex = (driver as any).knex; + const cols = await knex + .select('COLUMN_NAME', 'GENERATION_EXPRESSION') + .from('information_schema.COLUMNS') + .where({ TABLE_SCHEMA: knex.client.database(), TABLE_NAME: table }); + const idx = await knex + .select('INDEX_NAME', 'NON_UNIQUE', 'COLUMN_NAME') + .from('information_schema.STATISTICS') + .where({ TABLE_SCHEMA: knex.client.database(), TABLE_NAME: table }); + return { cols, idx }; + }; + + /** + * Is the declared UNIQUE physically present and carried by its shadow? + * Read as a positive claim from `STATISTICS`, so "no drift" can never be + * satisfied by an index that simply is not there. + */ + const carriedUniquePresent = async (table: string, indexName: string) => { + const { idx } = await catalog(table); + const parts = idx.filter((i: any) => i.INDEX_NAME === indexName); + return ( + parts.length === 1 && + Number(parts[0].NON_UNIQUE) === 0 && + parts[0].COLUMN_NAME === hashShadowColumnFor(indexName) + ); + }; + + it('a freshly synced shadow-carried unique reports no destructive index drift', async () => { + driver = new SqlDriver(cell.config()); + const obj = orgUniqueOn('os13015_fresh'); + await driver.initObjects([obj]); + + // POSITIVE CONTROL first: the constraint really exists, carried by the + // shadow. Without this, an empty drift list proves nothing. + expect(await carriedUniquePresent('os13015_fresh', 'uniq_os13015_fresh_org_v')).toBe(true); + + const drift = await driver.detectManagedDrift([obj]); + const onIndex = drift.filter((d) => d.kind === 'index_mismatch' || d.kind === 'unmapped_index'); + expect(onIndex).toEqual([]); + // And the shadow COLUMN is still protected from the orphan-column pass — + // the half of the vocabulary that was already taught. + expect(drift.filter((d) => d.kind === 'unmapped_column')).toEqual([]); + }); + + /** + * The remedy pin. Even on a second boot — the runtime ledger empty again, + * which is the state the card measured — applying every entry the differ + * produces WITH `--allow-destructive` must leave the constraint standing. + * Before the fix this ran `recreate_index`: it dropped the UNIQUE, the + * re-sync failed on the surviving generated column, and the catalog read + * below found nothing. + */ + it('survives "os migrate apply --allow-destructive" over every reported entry', async () => { + driver = new SqlDriver(cell.config()); + const obj = orgUniqueOn('os13015_apply'); + await driver.initObjects([obj]); + await driver.disconnect(); + + // A SECOND driver: `runtimeCreatedIndexes` starts empty, so the ledger + // escape hatch cannot mask the differ's verdict. + driver = new SqlDriver(cell.config()); + await driver.initObjects([obj]); + const drift = await driver.detectManagedDrift([obj]); + await driver.applyMigrationEntries(drift, { allowDestructive: true }); + + expect(await carriedUniquePresent('os13015_apply', 'uniq_os13015_apply_org_v')).toBe(true); + // …and it still ENFORCES: two NULL-organization rows with one payload. + const knex = (driver as any).knex; + const V = 'q'.repeat(900); + await knex('os13015_apply').insert({ id: 'a', v: V, organization_id: null }); + await expect( + knex('os13015_apply').insert({ id: 'b', v: V, organization_id: null }), + ).rejects.toThrow(/duplicate/i); + }); + + /** + * The surviving generated column, isolated: drop the index by name (exactly + * what `recreate_index` does) and re-sync. The shadow column is still + * there, and the re-sync must RE-KEY it rather than fail on a duplicate + * column name. + */ + it('re-keys a surviving shadow column instead of failing the rebuild', async () => { + driver = new SqlDriver(cell.config()); + const obj = orgUniqueOn('os13015_survive'); + await driver.initObjects([obj]); + const knex = (driver as any).knex; + const indexName = 'uniq_os13015_survive_org_v'; + + await knex.raw(`ALTER TABLE \`os13015_survive\` DROP INDEX \`${indexName}\``); + // The column OUTLIVES the index — the whole mechanism of the defect. + const { cols } = await catalog('os13015_survive'); + expect(cols.filter((c: any) => isHashShadowColumn(c.COLUMN_NAME)).length).toBe(1); + expect(await carriedUniquePresent('os13015_survive', indexName)).toBe(false); + + await driver.initObjects([obj]); + expect(await carriedUniquePresent('os13015_survive', indexName)).toBe(true); + }); + + /** + * The direction a blind skip would have lost: a shadow hashing the RAW + * columns (what shipped before #12998) must still be reported AND must be + * repairable — the stale column is re-generated, not reused. + */ + it('reports and repairs a shadow that hashes the raw columns', async () => { + driver = new SqlDriver(cell.config()); + const obj = orgUniqueOn('os13015_stale'); + await driver.initObjects([obj]); + const knex = (driver as any).knex; + const indexName = 'uniq_os13015_stale_org_v'; + const shadow = hashShadowColumnFor(indexName); + + // Reproduce the pre-#12998 physical state: raw CONCAT, no COALESCE. + await knex.raw(`ALTER TABLE \`os13015_stale\` DROP INDEX \`${indexName}\``); + await knex.raw(`ALTER TABLE \`os13015_stale\` DROP COLUMN \`${shadow}\``); + await knex.raw( + `ALTER TABLE \`os13015_stale\` ` + + `ADD COLUMN \`${shadow}\` VARBINARY(32) GENERATED ALWAYS AS ` + + `(UNHEX(SHA2(CONCAT(\`organization_id\`, 0x1f, \`v\`), 256))) STORED, ` + + `ADD UNIQUE KEY \`${indexName}\` (\`${shadow}\`)`, + ); + const before = await catalog('os13015_stale'); + const staleCol = before.cols.find((c: any) => c.COLUMN_NAME === shadow); + expect(String(staleCol.GENERATION_EXPRESSION).toLowerCase()).not.toContain('coalesce'); + + // The differ must SEE it — a blind skip would report nothing here. + const drift = await driver.detectManagedDrift([obj]); + const found = drift.find((d) => (d.op as any).indexName === indexName); + expect(found, 'a raw-column shadow is real drift and must be reported').toBeTruthy(); + expect(found!.op.type).toBe('recreate_index'); + + await driver.applyMigrationEntries(drift, { allowDestructive: true }); + + // Repaired in the catalog: the constraint stands and now folds NULL. + expect(await carriedUniquePresent('os13015_stale', indexName)).toBe(true); + const after = await catalog('os13015_stale'); + const fixed = after.cols.find((c: any) => c.COLUMN_NAME === shadow); + expect(String(fixed.GENERATION_EXPRESSION).toLowerCase()).toContain('coalesce'); + }); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 0a9f80a27e..5d47a18032 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -82,6 +82,10 @@ import { expectedIndexes, fieldHasColumn, GLOBAL_TENANT, + canonicalIndexKey, + hashShadowColumnFor, + isHashShadowCarrier, + parseHashShadowKeyParts, indexedKeyColumns, isIndexDriftOp, isUniqueScopeDeclared, @@ -91,6 +95,7 @@ import { parseIndexDdl, uniqueIndexesFromFields, type DeclaredIndexInput, + type HashShadowKeyPart, type ManagedDriftEntry, type DriftOp, type PhysicalIndex, @@ -11205,6 +11210,7 @@ export class SqlDriver implements IDataDriver { if (r.COLUMN_NAME != null) entry.columns.push(r.COLUMN_NAME); else if (r.EXPRESSION != null) applyIndexKeyParts(entry, [String(r.EXPRESSION)]); } + await this.resolveHashShadowKeys(tableName, [...byName.values()]); } } catch (e) { // Only a caller that can CORRECT a short read may ask for one (#7332). @@ -11213,6 +11219,65 @@ export class SqlDriver implements IDataDriver { return [...byName.values()]; } + /** + * Record, on each #11627 hash-shadow CARRIER, the declared key its shadow + * actually hashes (#13015). + * + * ## Why introspection and not the differ + * + * A carrier is recognisable by name alone — its sole key column is + * {@link SqlDriver.hashShadowColumnFor} of its own index name — but a NAME + * cannot say WHAT is hashed, and that difference decides between two + * opposite verdicts. A shadow written since #12998 hashes the declared key + * with its NULL-safe `COALESCE(organization_id, '__global__')` parts intact + * and is HEALTHY; one written before it hashes the RAW columns, so `CONCAT` + * yields NULL for every NULL-organization row and the constraint those rows + * were promised does not exist (#5030's shape). Both look identical from the + * `STATISTICS` view. Only the stored `GENERATION_EXPRESSION` separates them, + * and reading it is the difference between the differ SKIPPING shadows and + * the differ UNDERSTANDING them. + * + * ## Why the failure is swallowed + * + * This is an ENRICHMENT of a read with other consumers — the upsert + * conflict-target check and the boot's index-presence probe among them — and + * none of them may start failing because a second query did. A carrier whose + * key stays unresolved is still recognised as a carrier by + * `isHashShadowCarrier`, and `diffManagedIndexes` then reports NOTHING for it + * rather than proposing a rebuild it cannot reason about. The degraded + * outcome is a missing finding, never a dropped constraint. + */ + protected async resolveHashShadowKeys( + tableName: string, + indexes: PhysicalIndex[], + ): Promise { + const carriers = indexes.filter((i) => isHashShadowCarrier(i)); + if (carriers.length === 0) return; + try { + const rows: Array> = await this.knex + .select('COLUMN_NAME', 'GENERATION_EXPRESSION') + .from('information_schema.COLUMNS') + .where({ TABLE_SCHEMA: this.knex.client.database(), TABLE_NAME: tableName }) + .whereIn( + 'COLUMN_NAME', + carriers.map((i) => i.columns[0]!), + ); + const exprByColumn = new Map(); + for (const r of rows) { + const column = String(r.COLUMN_NAME ?? r.column_name ?? ''); + if (column) { + exprByColumn.set(column, String(r.GENERATION_EXPRESSION ?? r.generation_expression ?? '')); + } + } + for (const i of carriers) { + const parts = parseHashShadowKeyParts(exprByColumn.get(i.columns[0]!) ?? ''); + if (parts.length > 0) i.shadowKey = parts; + } + } catch { + /* see the head note: an unresolved carrier degrades to "no finding". */ + } + } + /** * Names of the indexes that already exist on a table. Used to make * declared-index sync idempotent across repeated runs. @@ -14032,11 +14097,12 @@ export class SqlDriver implements IDataDriver { * get different shadows. */ protected static hashShadowColumnFor(indexName: string): string { - const direct = `${indexName}${SqlDriver.HASH_SHADOW_SUFFIX}`; - if (direct.length <= 64) return direct; - const digest = createHash('sha256').update(indexName).digest('hex').slice(0, 8); - const keep = 64 - SqlDriver.HASH_SHADOW_SUFFIX.length - digest.length - 1; - return `${indexName.slice(0, keep)}_${digest}${SqlDriver.HASH_SHADOW_SUFFIX}`; + // #13015: DELEGATES rather than re-deriving. The differ has to look for + // exactly the column the sync creates, and the two halves of that question + // lived in different modules — which is how a healthy shadow-carried + // UNIQUE came to be reported as destructive drift while the shadow column + // itself was protected as driver-owned. One derivation, both readers. + return hashShadowColumnFor(indexName); } /** @@ -14143,11 +14209,48 @@ export class SqlDriver implements IDataDriver { columns.length === 1 ? part(columns[0]!) : `CONCAT(${columns.map(part).join(', 0x1f, ')})`; - const sql = - `ALTER TABLE ${ref(tableName)} ` + - `ADD COLUMN ${ref(shadow)} VARBINARY(32) GENERATED ALWAYS AS (UNHEX(SHA2(${expr}, 256))) STORED, ` + - `ADD UNIQUE KEY ${ref(indexName)} (${ref(shadow)})`; - await this.knex.raw(sql); + const addColumn = + `ADD COLUMN ${ref(shadow)} VARBINARY(32) GENERATED ALWAYS AS (UNHEX(SHA2(${expr}, 256))) STORED`; + const addKey = `ADD UNIQUE KEY ${ref(indexName)} (${ref(shadow)})`; + // #13015: the shadow column OUTLIVES the index it carries. Dropping a + // UNIQUE key by name does not drop the generated column keyed by it, so + // every path that drops and re-syncs — `recreate_index` above all — arrives + // back here with the survivor still on the table. The unconditional + // `ADD COLUMN` then failed with a duplicate-COLUMN error, which is matched + // by NEITHER the "already exists" absorb (that spells index names) nor the + // unique-violation branch, so the apply ended with the constraint DROPPED + // and not re-created: an operator following the differ's own advice removed + // a live uniqueness guarantee. The survivor is therefore inspected, never + // assumed absent. + const state = await this.hashShadowColumnState(tableName, indexName, columns, nullSafeColumns); + if (state === 'foreign') { + // A real, non-generated column already owns the name. It is not ours to + // drop — it may hold data — so this route is refused and the caller falls + // through to the named #11374 refusal, which is the honest outcome. + this.logDurabilityFailure( + `[sql-driver] cannot carry UNIQUE index '${indexName}' on "${tableName}" on a hash shadow — the ` + + `column "${shadow}" already exists and is NOT a generated column, so it is not the driver's to ` + + `replace (#11627/#13015). The declared constraint is NOT enforced; rename or drop that column.`, + ); + return false; + } + if (state === 'reusable') { + // The survivor already hashes exactly the declared key — re-key it rather + // than rebuild the table for a column that is byte-for-byte what we want. + await this.knex.raw(`ALTER TABLE ${ref(tableName)} ${addKey}`); + } else if (state === 'stale') { + // The survivor hashes a DIFFERENT key than metadata now declares — the + // pre-#12998 raw-column shadow is exactly this case. Reusing it would + // re-enforce the old constraint under the new name, which is the silent + // wrong answer; the column carries no user data (it is derived), so it is + // dropped and re-generated. Two statements rather than one ALTER: the + // index it carried is already gone by the time we are here, and a single + // `DROP COLUMN c, ADD COLUMN c` is a shape not worth relying on. + await this.knex.raw(`ALTER TABLE ${ref(tableName)} DROP COLUMN ${ref(shadow)}`); + await this.knex.raw(`ALTER TABLE ${ref(tableName)} ${addColumn}, ${addKey}`); + } else { + await this.knex.raw(`ALTER TABLE ${ref(tableName)} ${addColumn}, ${addKey}`); + } // The boot log describes the key the shadow actually enforces — the // NULL-safe parts in their COALESCE spelling — so "carried" can be read // literally (#12998). @@ -14158,12 +14261,72 @@ export class SqlDriver implements IDataDriver { `[sql-driver] UNIQUE index '${indexName}' on "${tableName}" is carried by the hash-shadow column ` + `"${shadow}" (SHA-256 of ${described}), because MySQL cannot key ${columns.length > 1 ? 'this column set' : 'a column'} ` + `longer than ${SqlDriver.MAX_KEYABLE_VARCHAR_CHARS} characters directly (#11627). The declared ` + - `constraint is enforced over the full value; only the physical key differs.`, - { tableName, indexName, columns, shadow }, + `constraint is enforced over the full value; only the physical key differs.` + + (state === 'reusable' + ? ` The shadow column already existed and hashes this exact key — re-keyed in place (#13015).` + : state === 'stale' + ? ` A surviving shadow column hashed a DIFFERENT key and was re-generated (#13015).` + : ''), + { tableName, indexName, columns, shadow, shadowColumnState: state }, ); return true; } + /** + * What is already sitting where this index's hash shadow goes (#13015)? + * + * - `absent` — nothing; create the column and the key together. + * - `reusable` — a generated column hashing EXACTLY the declared key. The + * index that keyed it was dropped (a `recreate_index`, a + * manual `DROP INDEX`, a half-applied migration); re-key it. + * - `stale` — a generated column hashing a DIFFERENT key. The pre-#12998 + * shadow over RAW columns is this case: reusing it would + * re-enforce the OLD constraint under the new name — green, + * silent, and wrong — so it is re-generated instead. + * - `foreign` — a column of that name that is not generated at all. Not + * ours; refuse rather than drop something that may hold data. + * + * "Same key" is decided by {@link canonicalIndexKey}, the differ's own + * identity for an index key, so the question this asks and the question + * `diffManagedIndexes` asks cannot answer differently. + * + * An unreadable catalog degrades to `absent` — exactly the behaviour that + * shipped before this probe existed — rather than converting a transient + * read failure into a refused constraint. + */ + protected async hashShadowColumnState( + tableName: string, + indexName: string, + columns: string[], + nullSafeColumns?: ReadonlySet, + ): Promise<'absent' | 'reusable' | 'stale' | 'foreign'> { + let rows: Array>; + try { + rows = await this.knex + .select('GENERATION_EXPRESSION') + .from('information_schema.COLUMNS') + .where({ + TABLE_SCHEMA: this.knex.client.database(), + TABLE_NAME: tableName, + COLUMN_NAME: SqlDriver.hashShadowColumnFor(indexName), + }); + } catch { + return 'absent'; + } + if (rows.length === 0) return 'absent'; + const expr = String(rows[0]?.GENERATION_EXPRESSION ?? rows[0]?.generation_expression ?? ''); + if (expr.trim() === '') return 'foreign'; + const parts = parseHashShadowKeyParts(expr); + if (parts.length === 0) return 'stale'; + const carried = canonicalIndexKey( + parts.map((k) => k.column), + parts.filter((k) => k.nullSafe).map((k) => k.column), + ); + return carried === canonicalIndexKey(columns, [...(nullSafeColumns ?? [])]) + ? 'reusable' + : 'stale'; + } + /** * Tell a genuine uniqueness violation apart from a hash COLLISION on a * shadow-carried UNIQUE index (#11627), and name which one happened. @@ -14312,7 +14475,7 @@ export class SqlDriver implements IDataDriver { protected async hashShadowSourceColumns( tableName: string, indexName: string, - ): Promise> { + ): Promise { try { const rows: Array<{ GENERATION_EXPRESSION?: string; generation_expression?: string }> = await this.knex @@ -14326,12 +14489,11 @@ export class SqlDriver implements IDataDriver { const expr = String(rows[0]?.GENERATION_EXPRESSION ?? rows[0]?.generation_expression ?? ''); // `unhex(sha2(`a`,256))`, `unhex(sha2(concat(`a`,0x1f,`b`),256))`, or with // a NULL-safe part: `…concat(coalesce(`org`,_utf8mb4'__global__'),0x1f,`b`)…` - // (#12998). The optional group marks which identifiers the expression - // wraps in COALESCE. - return [...expr.matchAll(/(coalesce\s*\(\s*)?`((?:[^`]|``)+)`/gi)].map((m) => ({ - column: m[2]!.replace(/``/g, '`'), - nullSafe: m[1] != null, - })); + // (#12998). #13015 moved the parse itself next to the shadow vocabulary + // in `schema-drift.ts`: the differ reads the same expression to decide + // whether a shadow-carried index enforces what metadata declares, and two + // copies of this regex would be two answers to one question. + return parseHashShadowKeyParts(expr); } catch { return []; }