Skip to content

Commit 0508e45

Browse files
dmealingclaude
andcommitted
fix(docs,migrate-ts): the remap hazard, measured instead of reasoned about
The previous commit claimed a member re-mapping is invisible to `meta migrate` and that a swap emits no migration at all. Probing the real diff says otherwise, and the corrected picture is both narrower and more useful: - The CHECK list renders in @values order, so ANY remap changes its text. That emits drop-check + add-check, and drop-check is BLOCKED by default (allow.dropCheck) — migrate refuses. The refusal is an ACCIDENT: it fires because dropping a CHECK is destructive, not because anything recognises that the meaning of stored data just changed. - Once allowed, the migration only refreshes the constraint and never touches the data. Moving a member to an unused int then applies a CHECK the existing rows violate and the database refuses it, loudly. SWAPPING two members leaves the admitted set identical, so it applies cleanly and every row has quietly changed meaning. - Only one shape is invisible to the diff: a remap plus a compensating @values reorder renders a byte-identical CHECK, so there is no diff to block. All three are now pinned in expected-schema-enum-intvaluemap.test.ts rather than asserted in prose — including the known gap, marked KNOWN so a future change that closes it updates the test instead of reverting the behaviour. Pinning the accident matters most: nothing else stops `allow.dropCheck` from being relaxed into a general auto-allow and taking the only protection with it. CHANGELOG gains the adopter-facing version: do not remap on a populated table; treat it as the same two-step backfill a backing-mode change needs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 7c9b65a commit 0508e45

3 files changed

Lines changed: 85 additions & 15 deletions

File tree

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,16 @@ that already has a column is a cross-kind `change-column-type`, which `meta migr
6767
blocks by default and requires an explicit `allow.typeChange` to pass. There is no
6868
auto-recast — the tool will not rewrite your data behind a metadata edit.
6969

70+
**Do not RE-map a member's integer on a populated table.** Nothing understands that change:
71+
the column holds bare integers, and neither introspection nor the committed snapshot records
72+
which member an integer stood for. A remap changes the rendered `CHECK` list, so it trips the
73+
blocked `drop-check` and `meta migrate` refuses — but that refusal is incidental (dropping a
74+
`CHECK` is destructive), and once allowed the migration only refreshes the constraint and
75+
never touches your rows. Swap two members' integers and the new `CHECK` admits the same set,
76+
applies cleanly, and every stored row has quietly changed meaning. Reorder `@values` to
77+
compensate and the diff is empty outright. Treat a remap as the same two-step backfill a
78+
backing-mode change needs.
79+
7080
Design: `docs/superpowers/specs/2026-07-23-int-backed-enum-values-design.md`. Adopter view:
7181
[`docs/features/field-types.md`](docs/features/field-types.md).
7282

docs/superpowers/specs/2026-07-23-int-backed-enum-values-design.md

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -267,21 +267,33 @@ just golden-snapshot codegen.
267267

268268
- Safe backing-mode migration (varchar↔integer recast with data preservation) — no
269269
current consumer; D8's manual path covers the only known need.
270-
- **RE-mapping an existing member's integer is not detected, and one shape of it is
271-
silent.** D8 covers ADDING or REMOVING `@intValueMap`; it does not cover changing a
272-
value inside a map that stays present. Two cases, only one of which is safe by
273-
accident: changing a member to an integer not already in the set (`DRAFT: 0` → `1`)
274-
alters the `CHECK`, so the migration applies a constraint every existing `0` row
275-
violates and the database refuses it — loud, at apply time. But **swapping two members'
276-
integers** (`DRAFT: 0, PUBLISHED: 5` → `DRAFT: 5, PUBLISHED: 0`) leaves the value SET
277-
identical, so the `CHECK` is byte-identical, the diff is EMPTY, no migration is emitted
278-
at all — and every stored row silently changes meaning. Nothing in the pipeline can see
279-
it: the column holds bare integers, and neither the introspected schema nor the
280-
committed schema snapshot records which member an integer stood for. Closing it needs
281-
the mapping itself carried in gen-state or the snapshot so a diff can compare
282-
member→int pairs rather than the value set — a design decision, not a patch. No current
283-
consumer remaps; documented here rather than left implied by D8's "migration safety"
284-
heading.
270+
- **RE-mapping an existing member's integer is not understood by anything; what saves you
271+
is incidental.** D8 covers ADDING or REMOVING `@intValueMap`, not changing a value inside
272+
a map that stays present. Measured against the real diff rather than reasoned about:
273+
274+
- **A remap that changes the rendered `CHECK` list** — which is every remap, since the
275+
list renders in `@values` order — emits `drop-check` + `add-check`, and the `drop-check`
276+
is **BLOCKED by default** (`allow.dropCheck`). So `meta migrate` refuses. That refusal
277+
is a happy accident: it fires because dropping a `CHECK` is destructive, not because
278+
anything recognises that the meaning of stored data just changed.
279+
- **Once allowed, the migration only refreshes the constraint — it never touches the
280+
data.** Moving a member to an int not already in the set (`DRAFT: 0` → `1`) then applies
281+
a `CHECK` every existing `0` row violates, and the database refuses it: loud, at apply
282+
time. But **swapping** two members' ints (`DRAFT: 0, PUBLISHED: 5` → `5, 0`) leaves the
283+
admitted SET identical, so the new `CHECK` applies cleanly and every stored row has
284+
quietly changed meaning.
285+
- **One shape is invisible even to the diff:** a remap combined with a compensating
286+
`@values` reorder renders a byte-identical `CHECK`, so the diff is EMPTY and no
287+
migration is emitted at all.
288+
289+
Nothing in the pipeline can see the meaning change in any of these: the column holds bare
290+
integers, and neither introspection nor the committed schema snapshot records which member
291+
an integer stood for. Closing it needs the mapping itself carried in gen-state or the
292+
snapshot so a diff can compare member→int PAIRS rather than the value set — a design
293+
decision, not a patch. No current consumer remaps; documented here rather than left
294+
implied by D8's "migration safety" heading, and pinned by
295+
`expected-schema-enum-intvaluemap.test.ts` so the accident that currently protects the
296+
common case cannot be removed silently.
285297
- Value aliasing (`allow_alias`-style opt-out of the duplicate-value rejection) — no
286298
current consumer.
287299
- Native Postgres `CREATE TYPE ... AS ENUM` — unrelated to this design, still deferred

server/typescript/packages/migrate-ts/test/expected-schema-enum-intvaluemap.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,3 +203,51 @@ describe("int-backed enum — backing-mode change is blocked by the existing gua
203203
expect(r.changes.find((c) => c.kind === "change-column-type")).toBeUndefined();
204204
});
205205
});
206+
207+
// RE-mapping a member inside a map that stays present is a different animal from D8's
208+
// add/remove, and nothing in the pipeline understands it: the column holds bare integers,
209+
// and neither introspection nor the committed snapshot records which member an integer
210+
// stood for. What currently protects the common case is an ACCIDENT — dropping a CHECK is
211+
// destructive, so the remap trips `allow.dropCheck` on its way past. These tests pin that
212+
// accident (so it cannot be relaxed silently) and pin the one shape it does not cover.
213+
describe("int-backed enum — re-mapping a member's integer", () => {
214+
const MAP_SWAPPED = { DRAFT: 5, PUBLISHED: 0, ARCHIVED: 9 };
215+
const MAP_MOVED = { DRAFT: 1, PUBLISHED: 5, ARCHIVED: 9 };
216+
217+
async function remapDiff(afterMap: Record<string, number>, afterValues = VALUES) {
218+
const before = buildExpectedSchema(
219+
await loadJson(entityModel({ name: "status", "@values": VALUES, "@intValueMap": INT_MAP })),
220+
);
221+
const after = buildExpectedSchema(
222+
await loadJson(
223+
entityModel({ name: "status", "@values": afterValues, "@intValueMap": afterMap }),
224+
),
225+
);
226+
return diff(after, before, { dialect: "postgres" });
227+
}
228+
229+
// The CHECK list renders in @values order, so ANY remap changes its text — which is the
230+
// only reason migrate sees a remap at all.
231+
test("a swap trips the blocked drop-check rather than passing silently", async () => {
232+
const r = await remapDiff(MAP_SWAPPED);
233+
const drop = r.changes.find((c) => c.kind === "drop-check");
234+
expect(drop).toBeDefined();
235+
expect(drop!.status.state).toBe("blocked");
236+
expect(r.changes.find((c) => c.kind === "add-check")).toBeDefined();
237+
});
238+
239+
test("moving a member to an unused int behaves the same way", async () => {
240+
const r = await remapDiff(MAP_MOVED);
241+
expect(r.changes.find((c) => c.kind === "drop-check")!.status.state).toBe("blocked");
242+
});
243+
244+
// The gap the two tests above do NOT cover: reorder @values to compensate for the swap
245+
// and the rendered CHECK is byte-identical, so there is no diff to block. Every stored
246+
// row changes meaning with no migration emitted at all. Pinned as KNOWN, not as desired
247+
// — if a future change makes this produce a diff, this test should be updated, not the
248+
// behaviour reverted.
249+
test("KNOWN GAP: a compensating @values reorder makes the remap invisible to the diff", async () => {
250+
const r = await remapDiff(MAP_SWAPPED, ["PUBLISHED", "DRAFT", "ARCHIVED"]);
251+
expect(r.changes).toEqual([]);
252+
});
253+
});

0 commit comments

Comments
 (0)