Skip to content

Commit 3f33d86

Browse files
dmealingclaude
andcommitted
fix(migrate): offline --allow adopt-view drop+creates a structural projection change (#240)
Follow-up to #239. The 0.20.4 fix keyed the legal/illegal CREATE OR REPLACE decision on BOTH column sets being known. But the OFFLINE (snapshot-based) migrate path diffs against a pre-fingerprint snapshot that records NO view columns, so `actual.columns` is undefined and the adopt branch fell back to a non-destructive replace-view — an illegal CREATE OR REPLACE for a projection whose desired shape is fully known (structural change → rejected at apply). The decision now keys on the EXPECTED (desired) view's columns only: a known projection runs `viewReplaceIsLegal`, which fail-safes to `false` (→ drop+create) when the actual columns are unknown too — exactly the offline adopt case. An opaque `@sql` body (expected columns unknown) still keeps its non-destructive replace on adoption (#208). #239 online + real-PG round-trip and the #208 @SQL adopt (view-lifecycle-pg 17/17) are unchanged. Diagnosed by reproducing the offline path: buildProjectionViews/buildExpectedSchema DO populate expected columns; it's the snapshot ACTUAL side that lacks them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014xy8powhHYJ6gfFt9Ut8dL
1 parent 6cf9f15 commit 3f33d86

3 files changed

Lines changed: 51 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,22 @@ here. The format follows [Keep a Changelog](https://keepachangelog.com/), and
55
this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html)
66
(pre-1.0; MINOR bumps may introduce breaking changes with notice).
77

8+
## [Unreleased]
9+
10+
### Fixed — offline `--allow adopt-view` illegal `CREATE OR REPLACE` for a projection (#240)
11+
12+
**npm-only** (`migrate-ts`). Follow-up to #239: the 0.20.4 fix keyed the legal/illegal
13+
`CREATE OR REPLACE` decision on **both** column sets being known, but the OFFLINE
14+
(snapshot-based) migrate path — the primary authoring path — diffs against a pre-fingerprint
15+
snapshot that records **no view columns**, so `actual.columns` is undefined and the adopt
16+
branch fell back to a non-destructive `replace-view` (= illegal `CREATE OR REPLACE`) even for
17+
a projection whose desired shape is fully known. The decision now keys on the **expected**
18+
(desired) view's columns only: a known projection runs `viewReplaceIsLegal` (which fail-safes
19+
to drop+create when the actual columns are unknown), so a structural change drop+creates
20+
offline too; an opaque `@sql` body (expected columns unknown) still keeps its non-destructive
21+
replace on adoption. Gated by a diff unit test (expected-known / actual-columns-absent →
22+
drop+create, still gated on `adoptView`); #208 `@sql` adopt + #239 real-PG round-trip unchanged.
23+
824
## [0.20.4] — 2026-07-26
925

1026
**npm-only** (NuGet `0.19.4` / PyPI `0.19.6` / Maven Central `7.11.4` unchanged — schema

server/typescript/packages/migrate-ts/src/diff/index.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -613,15 +613,20 @@ function pushViewUpdate(
613613
): void {
614614
const sx = schemaSpread(expected.schema);
615615
const adopt = unmanaged ? { unmanagedActual: true as const } : {};
616-
// #239: emit a replace only when it is a legal CREATE OR REPLACE. When both column
617-
// sets are KNOWN (a projection), that's viewReplaceIsLegal (a prefix-extension) — a
618-
// structural change (rename/reorder/mid-insert) is PROVABLY illegal and must be
619-
// drop+create. When the columns are UNKNOWN (an opaque `@sql` body), legality is
620-
// unprovable: a MANAGED body change still drops+creates (a real change is safest
621-
// rebuilt), but an ADOPTION keeps the non-destructive replace — the common
622-
// "re-stamp an identical, pre-fingerprint view" case (its behavior before #239).
616+
// #239/#240: emit a replace only when it is a legal CREATE OR REPLACE. The decision
617+
// keys on the EXPECTED (desired) view's column knowledge, NOT on both sides:
618+
// - EXPECTED columns KNOWN (a projection): run viewReplaceIsLegal. It fails safe to
619+
// `false` (→ drop+create) when the ACTUAL columns are unknown too — which is
620+
// exactly the OFFLINE adopt case (#240): a pre-fingerprint snapshot records no
621+
// view columns, yet the projection's desired shape IS known, so a structural
622+
// change must still drop+create, never an illegal OR REPLACE. (Also fails safe
623+
// when actual is known but non-prefix — the #239 online case.)
624+
// - EXPECTED columns UNKNOWN (an opaque `@sql` body): legality is unprovable. A
625+
// MANAGED change drops+creates (a real body change is safest rebuilt); an ADOPTION
626+
// keeps the non-destructive replace (the common "re-stamp an identical,
627+
// pre-fingerprint hand-written view" case, #208 — its behavior before #239).
623628
const legal =
624-
expected.columns !== undefined && actual.columns !== undefined
629+
expected.columns !== undefined
625630
? viewReplaceIsLegal(expected.columns, actual.columns)
626631
: unmanaged;
627632
if (legal) {

server/typescript/packages/migrate-ts/test/unit/diff.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,28 @@ describe("diff — Postgres adopt-view legality (#239)", () => {
212212
expect(r.changes.find((c) => c.kind === "drop-view")).toBeUndefined();
213213
});
214214

215+
test("#240 offline adopt: expected columns KNOWN but actual (old snapshot) has NO columns → drop+create", async () => {
216+
// A pre-fingerprint snapshot records no view columns (actual.columns undefined), but
217+
// a projection's desired shape IS known. The decision keys on EXPECTED knowledge, so
218+
// a structural change still drop+creates — never an illegal CREATE OR REPLACE.
219+
const actualNoCols = { name: "v_foo", sql: "SELECT t.a AS a, t.b AS b FROM t" }; // no columns, no fingerprint
220+
const r = await diff(
221+
{ tables: [], views: [expView(["a", "c"])] }, // expected knows [a, c] (b renamed to c)
222+
{ tables: [], views: [actualNoCols] },
223+
opts(true),
224+
);
225+
expect(r.changes.find((c) => c.kind === "replace-view")).toBeUndefined();
226+
expect(r.changes.find((c) => c.kind === "drop-view")).toMatchObject({ kind: "drop-view", view: "v_foo" });
227+
expect(r.changes.find((c) => c.kind === "create-view")).toBeDefined();
228+
// Still gated on adoptView (unmanagedActual on the drop half).
229+
const rBlocked = await diff(
230+
{ tables: [], views: [expView(["a", "c"])] },
231+
{ tables: [], views: [actualNoCols] },
232+
opts(false),
233+
);
234+
expect(rBlocked.changes.find((c) => c.kind === "drop-view")?.status.state).toBe("blocked");
235+
});
236+
215237
test("OPAQUE @sql body (columns unknown) adopts via non-destructive replace-view, not drop+create", async () => {
216238
// A hand-written @sql view has no parsed columns, so replace legality is unprovable.
217239
// The common case is re-stamping an identical pre-fingerprint view (#208), which a

0 commit comments

Comments
 (0)