diff --git a/.changeset/orphan-hash-shadow-column-cleanup.md b/.changeset/orphan-hash-shadow-column-cleanup.md new file mode 100644 index 0000000000..33879bfff5 --- /dev/null +++ b/.changeset/orphan-hash-shadow-column-cleanup.md @@ -0,0 +1,13 @@ +--- +'@objectstack/driver-sql': patch +--- + +Retiring a shadow-carried UNIQUE index no longer leaves its generated column behind forever (#13056). + +`isHashShadowColumn`'s docblock is why the orphan-COLUMN drift pass skips a #11627 hash shadow, and it stated what happens instead: the column "is then cleaned up by the index's own removal path, not by a blind column drop". There was no such path. `dropIndexIfExists` issues one statement family — `ALTER TABLE .. DROP CONSTRAINT`, `DROP INDEX IF EXISTS`, `ALTER TABLE .. DROP INDEX` — and never touches a column. So when metadata stopped declaring the index, `diffManagedIndexes` reported it as an orphan, `os migrate apply --allow-destructive` dropped it, and the `VARBINARY(32)` STORED generated column survived keyed by nothing, while the orphan-column pass declined to report it forever, exactly as designed. A STORED generated column is recomputed and written on every INSERT and on every UPDATE touching its source columns, so a table accumulating retired declarations paid for them permanently and silently. + +The `drop_index` op now collects that column after dropping the index. Ownership is established first, never assumed — in the shape of #13015's `foreign` guard, a column the driver has not proved is its own is left in place and named in a warning rather than dropped: a column of that name that is **not generated** may hold user data, and a column some **other index still keys** is not this orphan (that second read is what makes "index first, then column" a checked precondition rather than an ordering comment). An unreadable catalog degrades to leaving the column alone. + +**Why the cleanup hangs off the op and not off `dropIndexIfExists`,** which has two other callers. The discriminator is not *which caller* but *is this index name coming back*, and only the op knows. `recreate_index` drops in order to re-create under the same name, and its shadow must survive: #13015's `reusable` branch re-keys the survivor in place instead of rebuilding the table around a regenerated STORED column, and a cleanup in the shared helper would destroy exactly that survivor on every rebuild. `replace_unique_index`'s legacy-name drop cannot reach a shadow at all — #13015 already excludes `isHashShadowCarrier` from legacy detection, in `diffManagedIndexes`, saying it does so *because* that op drops the legacy name. Both are pinned in the negative direction, since they are what a later move of the drop into the shared helper would break and nothing else would notice. + +**Why `patch` and not `minor`.** Nothing new is authorable, no export is added (the collector is `protected`), and no input that was accepted is now rejected or vice versa. What an operator will observe that they did not before is a `DROP COLUMN` in the applied set of a migration they had already opted into: the `drop_index` op was already `category: 'destructive'` and already required `--allow-destructive`, so the opt-in is unchanged — the difference is that it now finishes the job it named instead of leaving half of it on the table. A `drop_index` that finds the index already gone is now reported as *applied* rather than skipped when it collects the leftover column, because the apply did rewrite the table. diff --git a/packages/drivers/driver-sql/src/schema-drift.ts b/packages/drivers/driver-sql/src/schema-drift.ts index 39f7154227..428a1b64c7 100644 --- a/packages/drivers/driver-sql/src/schema-drift.ts +++ b/packages/drivers/driver-sql/src/schema-drift.ts @@ -382,8 +382,21 @@ export const HASH_SHADOW_SUFFIX = '__hash'; * Matched by SUFFIX rather than by a registry of known names, deliberately: the * differ runs against a database whose metadata it is comparing to, and a * shadow whose declared index has since been removed must still be recognised - * as driver-owned (it is then cleaned up by the index's own removal path, not - * by a blind column drop). + * as driver-owned (it is cleaned up by the index's own removal path, not by a + * blind column drop). + * + * ⚠️ That last clause was an ASSERTION about a path that did not exist, and it + * is the load-bearing half of this docblock: it is the whole reason this pass + * may decline to report the column. #13056 built the path it names — + * `SqlDriver.dropOrphanedHashShadowColumn`, run by the `drop_index` op after + * the index goes — because until then `dropIndexIfExists` issued one statement + * family and never touched a column. The shadow of a retired declaration + * therefore outlived it forever: a `VARBINARY(32)` STORED generated column, + * recomputed on every INSERT and on every UPDATE touching its sources, keyed by + * nothing and reported by no pass. Skipping the column here is only correct + * while some other path really does collect it — so if that method is ever + * removed or its call site moved, this `continue` becomes a leak again and the + * skip must go with it. */ export function isHashShadowColumn(name: string): boolean { return name.endsWith(HASH_SHADOW_SUFFIX); diff --git a/packages/drivers/driver-sql/src/sql-driver-13056-orphan-shadow-column-cleanup.test.ts b/packages/drivers/driver-sql/src/sql-driver-13056-orphan-shadow-column-cleanup.test.ts new file mode 100644 index 0000000000..a90bb5a5ae --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-13056-orphan-shadow-column-cleanup.test.ts @@ -0,0 +1,575 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #13056 — the #11627 hash shadow of a RETIRED index is collected, so the + * cleanup `isHashShadowColumn` promises is a path that exists. + * + * ## The defect + * + * `isHashShadowColumn`'s docblock is why the orphan-COLUMN pass skips a shadow, + * and it stated what happens instead: the column "is then cleaned up by the + * index's own removal path, not by a blind column drop". There was no such + * path. `dropIndexIfExists` issues exactly one statement family — `ALTER TABLE + * .. DROP CONSTRAINT`, `DROP INDEX IF EXISTS`, `ALTER TABLE .. DROP INDEX` — + * and never touches a column. So when metadata stopped declaring a + * shadow-carried UNIQUE, `diffManagedIndexes` reported the index as an orphan, + * `os migrate apply --allow-destructive` dropped it, and the `VARBINARY(32)` + * STORED generated column survived keyed by nothing — while the orphan-column + * pass declined to report it forever, exactly as designed. A STORED generated + * column is recomputed and written on every INSERT and on every UPDATE touching + * its sources, so a table accumulating retired declarations pays for them + * permanently, invisibly. + * + * ## Why the cleanup is on the OP and not in `dropIndexIfExists` + * + * The discriminator is not "which caller" but "is this index name coming + * back", and only the op knows: + * + * - `drop_index` — TERMINAL. The declaration is gone; nothing will re-create + * the name, so the shadow derived from it is dead. This is the leak. + * - `recreate_index` — drops in order to re-create under the SAME name. Its + * shadow is deliberately kept: #13015's `reusable` branch re-keys the + * survivor in place instead of rebuilding the table around a regenerated + * STORED column. A cleanup inside `dropIndexIfExists` would destroy exactly + * that survivor on every rebuild. + * - `replace_unique_index` — cannot reach a shadow at all. #13015 already + * excludes `isHashShadowCarrier` from legacy detection, in `diffManagedIndexes`, + * with a comment saying it does so *because* that op drops the legacy name. + * A shadow-aware step there would be enforcement for a state the producer + * excludes by construction — coverage in appearance only. + * + * The last two are pinned below in the NEGATIVE direction on purpose: they are + * what a later "simplification" that moves the drop down into the shared helper + * would break, and nothing else would notice. + * + * ## What is read, and what is not + * + * The live cell reads the PHYSICAL catalog (`information_schema`), never the + * differ's report about itself, and carries a COLOCATED positive control: a + * second shadow-carried UNIQUE on the same table whose declaration is retained + * must be untouched by the same apply. A fix that dropped every shadow it found + * would pass the first assertion and fail the control. + * + * ⚠️ The live cell is OPT-IN and was NOT run while this was written — no MySQL + * is reachable in the authoring fleet. It is not a silent green: without + * `OS_TEST_MYSQL_URL` `declareDialectCell` declares a NAMED SKIP, and the + * runner that provisions the servers — the `Temporal Conformance (live PG + + * MySQL)` CI job, `ci.yml` step "Run driver-sql suite against both live + * servers" — sets `OS_EXPECT_LIVE_DIALECT_MATRIX=1`, which turns a missing URL + * into a failure. The dialect-free suites below therefore carry the pins that + * can execute anywhere: + * + * OS_TEST_MYSQL_URL=mysql://root:root@127.0.0.1:3306/conformance \ + * pnpm --filter @objectstack/driver-sql test + */ + +import { describe, it, expect, afterEach, beforeEach } from 'vitest'; +import { SqlDriver, diffManagedIndexes } from '../src/index.js'; +import { + hashShadowColumnFor, + isHashShadowCarrier, + type ExpectedIndex, + type LegacyUniqueReplacement, + type PhysicalIndex, +} from './schema-drift.js'; +import { MYSQL_CELL, declareDialectCell } from './live-dialect-matrix.testkit.js'; + +// ───────────────────────────────────────────────────────────────────────── +// 1. The legacy path is deliberately unchanged — because it cannot reach a +// shadow. Pinned at the PRODUCER, which is where the exclusion lives. +// ───────────────────────────────────────────────────────────────────────── + +describe('replace_unique_index never carries a shadow into its legacy drops (#13056)', () => { + const TABLE = 'os13056_legacy'; + /** The tenant composite that supersedes a legacy platform-wide unique. */ + const replacement: ExpectedIndex = { + name: 'uniq_os13056_legacy_organization_id_email', + columns: ['organization_id', 'email'], + unique: true, + }; + const legacyOn = (legacyName: string): LegacyUniqueReplacement => ({ + column: 'email', + legacyNames: [legacyName], + replacement, + legacyColumns: ['email'], + }); + + const diff = (legacy: LegacyUniqueReplacement[], physical: PhysicalIndex[]) => + diffManagedIndexes({ + table: TABLE, + expected: [replacement], + legacy, + physical, + tenantField: 'organization_id', + }); + + /** + * The POSITIVE CONTROL, and it is not a substring of the term under test: an + * ORDINARY legacy unique — a plain index physically keying the raw column — + * really is selected for the legacy drop. Without this, the shadow assertion + * below would be satisfied by a differ that had stopped emitting the op. + */ + it('selects an ordinary legacy unique for the drop', () => { + const legacyName = 'uniq_os13056_legacy_email'; + const entries = diff( + [legacyOn(legacyName)], + [{ name: legacyName, columns: ['email'], unique: true }], + ); + const replace = entries.filter((e) => e.op.type === 'replace_unique_index'); + expect(replace.length).toBe(1); + expect((replace[0]!.op as any).dropIndexNames).toEqual([legacyName]); + }); + + /** + * A shadow-carried UNIQUE is NOT a legacy shape, so it never enters + * `dropIndexNames` — which is the whole reason `applyIndexDriftOp`'s legacy + * loop needs no shadow-aware step and stays byte-identical. + */ + it('excludes a shadow carrier, so no shadow name can ever reach the drop list', () => { + const legacyName = 'uniq_os13056_legacy_email'; + const shadowCarrier: PhysicalIndex = { + name: legacyName, + columns: [hashShadowColumnFor(legacyName)], + unique: true, + }; + // The carrier really is one — the exclusion is doing the work, not a typo. + expect(isHashShadowCarrier(shadowCarrier)).toBe(true); + + const entries = diff([legacyOn(legacyName)], [shadowCarrier]); + for (const e of entries) { + if (e.op.type !== 'replace_unique_index') continue; + expect((e.op as any).dropIndexNames).not.toContain(legacyName); + } + // Stated positively too: the shadow-carried name is never proposed for the + // legacy drop under ANY op this call produced. + const dropped = entries.flatMap((e) => + e.op.type === 'replace_unique_index' ? (e.op as any).dropIndexNames : [], + ); + expect(dropped).toEqual([]); + }); +}); + +// ───────────────────────────────────────────────────────────────────────── +// 2. The apply path, against a STANDING-IN catalog +// ───────────────────────────────────────────────────────────────────────── + +interface FakeColumn { + name: string; + /** Empty string = a real, non-generated column (MySQL's own spelling). */ + generationExpression: string; +} +interface FakeIndexPart { + indexName: string; + column: string; +} + +/** + * A MySQL-flavoured driver whose `information_schema` reads and DDL come from + * an in-memory catalog. `isMysql` is derived from `config.client` and from + * nothing else, so overriding it is the whole of "pretend to be MySQL" — the + * same idiom `sql-driver-deferred-ddl-lock-wait` uses. + * + * Every statement is recorded IN ORDER, which is how "index first, then column" + * is asserted as an observation rather than assumed from the source. + */ +class CatalogProbeDriver extends SqlDriver { + columns: FakeColumn[] = []; + indexParts: FakeIndexPart[] = []; + statements: string[] = []; + logs: Array<{ level: string; msg: string }> = []; + pretendMysql = true; + /** Set to make the `information_schema` reads throw, as an unreachable catalog does. */ + catalogUnreadable = false; + + protected override get isMysql(): boolean { + return this.pretendMysql; + } + + protected override logger: any = { + warn: (msg: string) => this.logs.push({ level: 'warn', msg }), + error: (msg: string) => this.logs.push({ level: 'error', msg }), + info: () => {}, + debug: () => {}, + }; + + /** The index sync is not under test here; record the intent and stop. */ + protected override async syncDeclaredIndexes(table: string, indexes: any[]): Promise { + for (const i of indexes) this.statements.push(`SYNC ${table} ${i.name}`); + for (const i of indexes) this.indexParts.push({ indexName: i.name, column: i.fields[0] }); + } + + protected override async getExistingIndexNames(): Promise> { + return new Set(this.indexParts.map((p) => p.indexName)); + } + + installFakeKnex(): void { + const self = this; + const render = (sql: string, bindings?: any[]) => { + let i = 0; + return sql.replace(/\?\?/g, () => String(bindings?.[i++] ?? '?')); + }; + const fake: any = (table: string) => ({ + columnInfo: async () => + Object.fromEntries(self.columns.map((c) => [c.name, { type: 'varbinary' }])), + // Only reached by paths this suite does not exercise; loud rather than silent. + then: undefined, + _table: table, + }); + fake.client = { database: () => 'os13056_db' }; + fake.raw = async (sql: string, bindings?: any[]) => { + const rendered = render(sql, bindings); + self.statements.push(rendered); + const dropIndex = /DROP (?:INDEX|CONSTRAINT)(?: IF EXISTS)? (\S+)/i.exec(rendered); + if (dropIndex) { + const name = dropIndex[1]!; + self.indexParts = self.indexParts.filter((p) => p.indexName !== name); + } + const dropColumn = /DROP COLUMN (\S+)/i.exec(rendered); + if (dropColumn) { + const name = dropColumn[1]!; + self.columns = self.columns.filter((c) => c.name !== name); + } + return []; + }; + fake.select = (...cols: string[]) => ({ + from: (source: string) => ({ + where: async (w: Record) => { + if (self.catalogUnreadable) throw new Error('information_schema unreachable'); + if (/COLUMNS$/i.test(source)) { + return self.columns + .filter((c) => c.name === w.COLUMN_NAME) + .map((c) => ({ GENERATION_EXPRESSION: c.generationExpression })); + } + if (/STATISTICS$/i.test(source)) { + return self.indexParts + .filter((p) => p.column === w.COLUMN_NAME) + .map((p) => ({ INDEX_NAME: p.indexName })); + } + throw new Error(`unexpected catalog read: ${source} ${cols.join(',')}`); + }, + }), + }); + (this as any).knex = fake; + } + + apply(op: any): Promise { + return (this as any).applyIndexDriftOp(op); + } + collect(table: string, indexName: string): Promise { + return (this as any).dropOrphanedHashShadowColumn(table, indexName); + } +} + +describe('the drop_index op collects the shadow it retires (#13056)', () => { + const TABLE = 'os13056_apply'; + const RETIRED = 'uniq_os13056_apply_token'; + const SHADOW = hashShadowColumnFor(RETIRED); + /** What a healthy MySQL 8 shadow reports for `GENERATION_EXPRESSION`. */ + const GENERATED = 'unhex(sha2(`token`,256))'; + + let driver: CatalogProbeDriver; + let realKnex: any; + + beforeEach(() => { + driver = new CatalogProbeDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + realKnex = (driver as any).knex; + driver.installFakeKnex(); + driver.columns = [ + { name: 'id', generationExpression: '' }, + { name: 'token', generationExpression: '' }, + { name: SHADOW, generationExpression: GENERATED }, + ]; + driver.indexParts = [{ indexName: RETIRED, column: SHADOW }]; + }); + + afterEach(async () => { + (driver as any).knex = realKnex; + await driver.disconnect().catch(() => {}); + }); + + const names = () => driver.columns.map((c) => c.name); + + it('drops the index and THEN the column it keyed, in that order', async () => { + const applied = await driver.apply({ type: 'drop_index', table: TABLE, indexName: RETIRED }); + + expect(applied).toBe(true); + // The column is physically gone from the catalog — the direction the whole + // card is about. + expect(names()).not.toContain(SHADOW); + expect(names()).toEqual(['id', 'token']); + + const dropIndexAt = driver.statements.findIndex((s) => /DROP INDEX/i.test(s)); + const dropColumnAt = driver.statements.findIndex((s) => /DROP COLUMN/i.test(s)); + expect(dropIndexAt).toBeGreaterThanOrEqual(0); + expect(dropColumnAt).toBeGreaterThanOrEqual(0); + expect(dropIndexAt).toBeLessThan(dropColumnAt); + expect(driver.statements[dropColumnAt]).toBe(`ALTER TABLE ${TABLE} DROP COLUMN ${SHADOW}`); + }); + + /** + * The COLOCATED positive control for every "the column went" assertion above: + * a shadow belonging to an index that is still there is not this op's to + * collect, and the guard that says so is a catalog read, not the op's name. + */ + it('leaves a shadow whose index still exists untouched', async () => { + const KEPT = 'uniq_os13056_apply_secret'; + const KEPT_SHADOW = hashShadowColumnFor(KEPT); + driver.columns.push({ name: KEPT_SHADOW, generationExpression: 'unhex(sha2(`secret`,256))' }); + driver.indexParts.push({ indexName: KEPT, column: KEPT_SHADOW }); + + await driver.apply({ type: 'drop_index', table: TABLE, indexName: RETIRED }); + + expect(names()).not.toContain(SHADOW); + expect(names()).toContain(KEPT_SHADOW); + expect(driver.statements.filter((s) => /DROP COLUMN/i.test(s))).toEqual([ + `ALTER TABLE ${TABLE} DROP COLUMN ${SHADOW}`, + ]); + }); + + /** + * #13015's `foreign` guard is the precedent, and this is the same refusal in + * the removal direction: a column of that name that is not generated may hold + * user data and is not the driver's to drop. + */ + it('REFUSES a column of that name that is not a generated column', async () => { + driver.columns = [ + { name: 'id', generationExpression: '' }, + { name: SHADOW, generationExpression: '' }, + ]; + driver.indexParts = [{ indexName: RETIRED, column: SHADOW }]; + + const applied = await driver.apply({ type: 'drop_index', table: TABLE, indexName: RETIRED }); + + // The index still went — only the column was spared. + expect(applied).toBe(true); + expect(names()).toContain(SHADOW); + expect(driver.statements.filter((s) => /DROP COLUMN/i.test(s))).toEqual([]); + expect(driver.logs.some((l) => /NOT dropping/.test(l.msg) && /NOT a generated column/.test(l.msg))).toBe(true); + }); + + /** + * "Index first, then column" as a CHECKED precondition rather than an + * ordering comment: something still keying the column means either the drop + * did not take or a second index would be removed as a side effect. Either + * way this is not the orphan being collected. + */ + it('REFUSES while another index still keys the column', async () => { + driver.indexParts.push({ indexName: 'idx_os13056_apply_manual', column: SHADOW }); + + await driver.apply({ type: 'drop_index', table: TABLE, indexName: RETIRED }); + + expect(names()).toContain(SHADOW); + expect(driver.statements.filter((s) => /DROP COLUMN/i.test(s))).toEqual([]); + expect( + driver.logs.some((l) => /still key that column/.test(l.msg) && /idx_os13056_apply_manual/.test(l.msg)), + ).toBe(true); + }); + + it('collects a column the index drop already removed — the half-applied migration', async () => { + // The index is gone (a previous, partial apply); only the leak is left. + driver.indexParts = []; + + const applied = await driver.apply({ type: 'drop_index', table: TABLE, indexName: RETIRED }); + + expect(names()).not.toContain(SHADOW); + // `dropIndexIfExists` found nothing, yet the apply DID rewrite the table. + expect(applied).toBe(true); + }); + + it('is a no-op when no shadow survived', async () => { + driver.columns = [ + { name: 'id', generationExpression: '' }, + { name: 'token', generationExpression: '' }, + ]; + + await driver.apply({ type: 'drop_index', table: TABLE, indexName: RETIRED }); + + expect(driver.statements.filter((s) => /DROP COLUMN/i.test(s))).toEqual([]); + expect(driver.logs.filter((l) => /NOT dropping/.test(l.msg))).toEqual([]); + }); + + it('degrades to leaving the column alone when the catalog cannot be read', async () => { + driver.catalogUnreadable = true; + + await expect( + driver.apply({ type: 'drop_index', table: TABLE, indexName: RETIRED }), + ).resolves.toBe(true); + expect(names()).toContain(SHADOW); + expect(driver.statements.filter((s) => /DROP COLUMN/i.test(s))).toEqual([]); + }); + + it('reads no catalog and drops no column on a non-MySQL dialect', async () => { + driver.pretendMysql = false; + + await driver.apply({ type: 'drop_index', table: TABLE, indexName: RETIRED }); + + expect(names()).toContain(SHADOW); + expect(driver.statements.filter((s) => /DROP COLUMN/i.test(s))).toEqual([]); + }); + + // ── The two NEGATIVE pins: the other callers of `dropIndexIfExists` ────── + + /** + * `recreate_index` drops and re-creates under the SAME name. Its shadow must + * SURVIVE, or #13015's `reusable` branch — which re-keys the survivor in + * place rather than rebuilding the table around a regenerated STORED column — + * can never be reached again. This is the assertion a cleanup moved into + * `dropIndexIfExists` would fail. + */ + it('leaves the shadow in place across a recreate_index', async () => { + await driver.apply({ + type: 'recreate_index', + table: TABLE, + indexName: RETIRED, + columns: ['token'], + unique: true, + }); + + expect(names()).toContain(SHADOW); + expect(driver.statements.filter((s) => /DROP COLUMN/i.test(s))).toEqual([]); + expect(driver.statements.some((s) => /DROP INDEX/i.test(s))).toBe(true); + }); + + /** + * `replace_unique_index`'s legacy drop stays byte-identical. Suite 1 pins + * that a shadow can never REACH this list; this pins that the apply would not + * act on one even if it somehow did — so the two halves of the answer to + * "should a legacy name's shadow go with it?" are both executable. + */ + it('leaves the shadow in place across a replace_unique_index legacy drop', async () => { + await driver.apply({ + type: 'replace_unique_index', + table: TABLE, + dropIndexNames: [RETIRED], + createIndexName: 'uniq_os13056_apply_organization_id_token', + createColumns: ['organization_id', 'token'], + }); + + expect(names()).toContain(SHADOW); + expect(driver.statements.filter((s) => /DROP COLUMN/i.test(s))).toEqual([]); + }); +}); + +// ───────────────────────────────────────────────────────────────────────── +// 3. Live MySQL: the PHYSICAL catalog, after `os migrate apply` runs +// ───────────────────────────────────────────────────────────────────────── + +/** + * Two full-value UNIQUE indexes over columns MySQL cannot key directly, so both + * take the #11627 shadow route. Retiring ONE declaration puts the control and + * the subject on the same table, in the same apply. + */ +const twoCarriedUniques = (name: string, indexes: Array<'retired' | 'kept'>) => ({ + name, + fields: { + retired: { type: 'text', maxLength: 1024 }, + kept: { type: 'text', maxLength: 1024 }, + }, + indexes: indexes.map((f) => ({ fields: [f], unique: true as const, name: `uniq_${name}_${f}` })), +}); + +declareDialectCell(MYSQL_CELL, 'orphan shadow column cleanup (#13056)', (cell) => { + describe('a retired shadow-carried UNIQUE leaves no column behind (#13056)', () => { + 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 { + columns: cols.map((c: any) => String(c.COLUMN_NAME)), + indexes: idx.map((i: any) => String(i.INDEX_NAME)), + }; + }; + + it('drops the orphaned generated column, and keeps the one still carrying a constraint', async () => { + const TABLE = 'os13056_orphan'; + const retiredIndex = `uniq_${TABLE}_retired`; + const keptIndex = `uniq_${TABLE}_kept`; + const retiredShadow = hashShadowColumnFor(retiredIndex); + const keptShadow = hashShadowColumnFor(keptIndex); + + driver = new SqlDriver(cell.config()); + await driver.initObjects([twoCarriedUniques(TABLE, ['retired', 'kept'])]); + await driver.disconnect(); + + // POSITIVE CONTROL, before anything is retired: both shadows exist and + // both indexes really are there. An "it is gone" assertion below proves + // nothing without this. + driver = new SqlDriver(cell.config()); + await driver.initObjects([twoCarriedUniques(TABLE, ['retired', 'kept'])]); + const before = await catalog(TABLE); + expect(before.columns).toContain(retiredShadow); + expect(before.columns).toContain(keptShadow); + expect(before.indexes).toContain(retiredIndex); + expect(before.indexes).toContain(keptIndex); + await driver.disconnect(); + + // A FRESH driver so `runtimeCreatedIndexes` starts empty — the ledger + // escape hatch cannot mask the differ's verdict. Metadata now declares + // only `kept`. + const retired = twoCarriedUniques(TABLE, ['kept']); + driver = new SqlDriver(cell.config()); + await driver.initObjects([retired]); + const drift = await driver.detectManagedDrift([retired]); + const orphan = drift.find((d) => (d.op as any).indexName === retiredIndex); + expect(orphan, 'the retired index must be reported as an orphan').toBeTruthy(); + expect(orphan!.op.type).toBe('drop_index'); + + await driver.applyMigrationEntries(drift, { allowDestructive: true }); + + const after = await catalog(TABLE); + // The subject: index AND its generated column are both gone. + expect(after.indexes).not.toContain(retiredIndex); + expect(after.columns).not.toContain(retiredShadow); + // The control: the still-declared constraint and its column are intact. + expect(after.indexes).toContain(keptIndex); + expect(after.columns).toContain(keptShadow); + + // …and the surviving constraint still BITES, so "kept" means enforced. + const knex = (driver as any).knex; + const V = 'k'.repeat(900); + await knex(TABLE).insert({ id: 'a', kept: V }); + await expect(knex(TABLE).insert({ id: 'b', kept: V })).rejects.toThrow(/duplicate/i); + }); + + /** + * The differ must go quiet afterwards for the right reason: with the column + * collected there is no orphan column for the pass to skip, and a second + * apply has nothing left to do. + */ + it('converges — a second detect finds neither the index nor an orphan column', async () => { + const TABLE = 'os13056_converge'; + driver = new SqlDriver(cell.config()); + await driver.initObjects([twoCarriedUniques(TABLE, ['retired', 'kept'])]); + await driver.disconnect(); + + const retired = twoCarriedUniques(TABLE, ['kept']); + driver = new SqlDriver(cell.config()); + await driver.initObjects([retired]); + await driver.applyMigrationEntries(await driver.detectManagedDrift([retired]), { + allowDestructive: true, + }); + + const again = await driver.detectManagedDrift([retired]); + expect(again.filter((d) => d.kind === 'unmapped_index')).toEqual([]); + expect(again.filter((d) => d.kind === 'unmapped_column')).toEqual([]); + const after = await catalog(TABLE); + expect(after.columns).not.toContain(hashShadowColumnFor(`uniq_${TABLE}_retired`)); + }); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 33328ff96b..0adb4e25f6 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -10788,8 +10788,34 @@ export class SqlDriver implements IDataDriver { // failures (a NULL-safe unique over data that still violates it) into // a loud log instead of a throw, so presence is the only proof. return (await this.getExistingIndexNames(op.table)).has(op.indexName); - case 'drop_index': - return await this.dropIndexIfExists(op.table, op.indexName); + case 'drop_index': { + // #13056: a TERMINAL removal — the name is retired, nothing will + // re-declare it, and the #11627 hash shadow derived from that name is + // dead the moment the index goes. Index FIRST, then the column it + // keyed: MySQL refuses to drop a column a key still uses, and the + // reverse order would take the constraint down with the column while + // the operator was told only that an index was dropped. + // + // ⚠️ Deliberately HERE and not inside `dropIndexIfExists`. That helper + // has two other callers and neither is terminal: `recreate_index` + // drops in order to re-create under the SAME name — routing it through + // this cleanup would destroy the survivor #13015's `reusable` branch + // exists to re-key in place, turning every rebuild into a full + // regeneration of a STORED generated column — and + // `replace_unique_index` cannot reach a shadow at all (its legacy + // detection excludes `isHashShadowCarrier`, stated outright in + // `diffManagedIndexes` precisely because that op DROPS the legacy + // name). "Which caller" is the wrong axis; "is this name coming back" + // is the right one, and only the op knows. + const dropped = await this.dropIndexIfExists(op.table, op.indexName); + // Applied when EITHER half changed the table. A half-applied migration + // leaves the index gone and the column behind, so an already-absent + // index is exactly the state whose leftover column still needs + // collecting — reporting that as `skipped` would claim the apply did + // nothing to a table it had just rewritten. + const shadowDropped = await this.dropOrphanedHashShadowColumn(op.table, op.indexName); + return dropped || shadowDropped; + } case 'recreate_index': { // Same name on both sides — the drop has to come first, and a UNIQUE // target can fail on existing duplicates. That is why this op is @@ -14433,6 +14459,118 @@ export class SqlDriver implements IDataDriver { : 'stale'; } + /** + * Collect the #11627 hash shadow an index left behind when its name was + * retired for good (#13056). + * + * ## The contract this makes true + * + * {@link isHashShadowColumn}'s docblock is why the ORPHAN-column pass skips a + * shadow: dropping it blindly would take the UNIQUE index it carries with it. + * That docblock also stated what happens instead — the column "is then + * cleaned up by the index's own removal path" — and until this method there + * was no such path. Measured: `dropIndexIfExists` issues one statement family + * (`DROP CONSTRAINT` / `DROP INDEX` / `ALTER TABLE .. DROP INDEX`) and never + * touches a column, so when metadata stopped declaring a shadow-carried + * UNIQUE the differ reported the index as an orphan, the apply dropped it, + * and the `VARBINARY(32)` STORED generated column survived with nothing + * keying it — while the orphan-column pass declined to report it forever, + * exactly as designed. A STORED generated column is recomputed and written on + * every INSERT and on every UPDATE touching its sources, so a table that + * accumulates retired declarations pays for them permanently. + * + * ## Ownership is ESTABLISHED, never assumed + * + * Two refusals, in the shape of #13015's `foreign` guard — a column we have + * not proved is ours is left alone and named in the log, never dropped: + * + * - a column of this name that is **not generated** may hold user data; + * - a column something **still keys** is not the orphan this collects. That + * second read is what turns "index first, then column" from an ordering + * comment into a checked precondition: it fails closed when the drop above + * silently did not take, and it declines to let a `DROP COLUMN` remove a + * second index as a side effect. + * + * An unreadable catalog degrades to "leave it alone" — the behaviour that + * shipped before this method existed — rather than turning a transient read + * failure into a dropped column, the same trade + * {@link hashShadowColumnState} makes. + * + * Returns `true` only when a column was actually dropped. + */ + protected async dropOrphanedHashShadowColumn( + tableName: string, + indexName: string, + ): Promise { + // Shadows are a MySQL-only construct: `createHashShadowUniqueIndex` refuses + // every other dialect, so there is nothing to collect and no catalog read + // worth paying for. + if (!this.isMysql) return false; + const shadow = SqlDriver.hashShadowColumnFor(indexName); + + let generationExpression: string; + try { + const rows = await this.knex + .select('GENERATION_EXPRESSION') + .from('information_schema.COLUMNS') + .where({ + TABLE_SCHEMA: this.knex.client.database(), + TABLE_NAME: tableName, + COLUMN_NAME: shadow, + }); + if (rows.length === 0) return false; // nothing survived; the ordinary case + generationExpression = String( + rows[0]?.GENERATION_EXPRESSION ?? rows[0]?.generation_expression ?? '', + ); + } catch { + return false; + } + if (generationExpression.trim() === '') { + this.logger.warn( + `[sql-driver] NOT dropping "${tableName}"."${shadow}" while collecting the retired index ` + + `'${indexName}' — a column of that name exists and is NOT a generated column, so it is not ` + + `the driver's to drop and may hold data (#11627/#13056). Remove it by hand if it is unwanted.`, + { tableName, indexName, shadow }, + ); + return false; + } + + let stillKeyedBy: string[]; + try { + const rows = await this.knex + .select('INDEX_NAME') + .from('information_schema.STATISTICS') + .where({ + TABLE_SCHEMA: this.knex.client.database(), + TABLE_NAME: tableName, + COLUMN_NAME: shadow, + }); + stillKeyedBy = [ + ...new Set(rows.map((r: any) => String(r.INDEX_NAME ?? r.index_name ?? ''))), + ].filter((n) => n !== ''); + } catch { + return false; + } + if (stillKeyedBy.length > 0) { + this.logger.warn( + `[sql-driver] NOT dropping "${tableName}"."${shadow}" while collecting the retired index ` + + `'${indexName}' — ${stillKeyedBy.length} index(es) still key that column ` + + `(${stillKeyedBy.join(', ')}), so dropping it would remove them too (#13056).`, + { tableName, indexName, shadow, stillKeyedBy }, + ); + return false; + } + + await this.knex.raw('ALTER TABLE ?? DROP COLUMN ??', [tableName, shadow]); + this.logger.warn( + `[sql-driver] dropped the hash-shadow column "${shadow}" from "${tableName}" — the UNIQUE index ` + + `'${indexName}' it carried is retired, so the STORED generated column was recomputed on every ` + + `write for a constraint that no longer exists (#11627/#13056).`, + { tableName, indexName, shadow }, + ); + return true; + } + /** * Tell a genuine uniqueness violation apart from a hash COLLISION on a * shadow-carried UNIQUE index (#11627), and name which one happened.