Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/orphan-hash-shadow-column-cleanup.md
Original file line numberDiff line numberDiff line change
@@ -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.
17 changes: 15 additions & 2 deletions packages/drivers/driver-sql/src/schema-drift.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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);
Expand Down
Loading
Loading