no-mistakes(review): fix(migrate-ts): recreate unchanged Postgres views over retyped untracked tables Pass 2c now covers Postgres: an unchanged view is dropped before and created after a table change when its stored body names an altered table outside dependsOn and the altered column (column-scoped). Unit case and an executed live Postgres case added; both fail without the fix. F2: the CHANGELOG bullet "SQLite and D1 are unchanged" now says the replace rule does not apply to SQLite and D1. MySQL: migrate-ts Dialect is postgres | sqlite | d1; no MySQL path exists, no change

Author: dmealing

CHANGELOG.md

@@ -1039,9 +1039,9 @@ until you regenerate.
   - **The snapshot no longer records a view's columns.** Nothing reads them. The format version
     stays `3`, and an older snapshot that carries them still loads; they are dropped on read.
     `ExpectedView.columns` is removed from `@metaobjectsdev/codegen-ts` for the same reason.
-  - SQLite and D1 are unchanged. Neither has `CREATE OR REPLACE VIEW`; a replaced view is
-    written as `DROP VIEW` and `CREATE VIEW` there, as it always was, so there is nothing to
-    refuse.
+  - The replace rule does not apply to SQLite and D1. Neither has `CREATE OR REPLACE VIEW`; a
+    replaced view is written as `DROP VIEW` and `CREATE VIEW` there, as it always was, so there
+    is nothing to refuse.
 - **The `--allow` hint for a blocked view change names a token the parser accepts.** It printed
   `--allow replace-view`, which `meta migrate` rejects (`invalid --allow token`). It names
   `adopt-view` for an unfingerprinted view and `drop-view-cascade` when the drop would cascade.
@@ -1051,7 +1051,11 @@ until you regenerate.
 - **A column change recreates only the views that read the column.** On Postgres a new default or a
   changed nullability no longer drops and rebuilds the views over the table (neither blocks a
   view), and a type change recreates only the views whose SQL names the column (a view that
-  selects `*` or joins `NATURAL` reads columns it does not name, and is always recreated). SQLite and D1 still
+  selects `*` or joins `NATURAL` reads columns it does not name, and is always recreated). That
+  now holds for a table the model does not list for the view as well: an unchanged `@sql` view
+  that joins such a table is dropped before and created after a retype of a column it reads,
+  where the migration failed at apply before (`cannot alter type of a column used by a view or
+  rule`). The table is found by name in the view's stored definition. SQLite and D1 still
   recreate every view over a rebuilt table, and so does a `diff()` called with no `dialect`,
   whose changes may be emitted for SQLite. `verify --db` and `meta migrate` now print the full
   column type in a type change (`text(255) -> text`, `timestamp -> timestamptz`), so a length or

docs/features/migrations-and-drift.md

@@ -464,6 +464,13 @@ A drop and a create of the same view needs no `--allow`. It has two costs a repl
 Plan with `--from-db` to get the in-place replace when it is legal, which keeps both. Otherwise
 grant the privileges again after the migration is applied.
 
+A view whose definition does **not** change is dropped and created too, when the migration
+retypes, drops or renames a column the view reads: Postgres refuses to retype or drop it
+while the view stands. The table may be one the model does not list for the view, which is
+the case of an `@sql` body that joins it. `meta migrate` finds it by name in the view's
+stored definition, and recreates the view only when that definition also names the changed
+column. The same two costs apply.
+
 SQLite and D1 have no `CREATE OR REPLACE VIEW`: every replaced view is written as `DROP VIEW`
 and `CREATE VIEW` there, so there is no refusal to avoid and no probe. They refuse something
 else: a `DROP COLUMN`, or the rebuild of a table, while a view that reads the table stands. So

server/typescript/packages/integration-tests/test/view-replace-probe-pg.test.ts

@@ -573,6 +573,41 @@ describe("an @sql body in a plan that changes a table's columns (postgres)", ()
     expect((await plan(after.expected)).changes).toEqual([]);
   }, 60_000);
 
+  // The view's definition does not change here, so nothing replaces it: it only has to be
+  // out of the way while the column it reads is retyped.
+  test("an unchanged body in a plan that retypes a column it reads is dropped and created, and applies", async () => {
+    const before = await model(scoredView(BEFORE.scored), SUMMARY_VIEW, [], BEFORE.others);
+    await migrate(before.expected);
+    const after = await model(scoredView(BEFORE.scored), SUMMARY_VIEW, [], [badge("long")]);
+    expect(after.view.fingerprint).toBe(before.view.fingerprint);
+    expect(after.view.dependsOn).toEqual(["programs"]);
+    await expect(sql.raw(`ALTER TABLE "badges" ALTER COLUMN "points" TYPE BIGINT`).execute(k))
+      .rejects.toThrow(/cannot alter type of a column used by a view or rule/);
+
+    const changed = await plan(after.expected);
+    expect(changed.blocked).toEqual([]);
+    expect(kindsOf(changed.changes)).toEqual(["change-column-type", "create-view", "drop-view"]);
+    await apply(changed.changes);
+
+    expect(await scalar(`SELECT format_type(atttypid, atttypmod) FROM pg_attribute
+                          WHERE attrelid = 'public.badges'::regclass AND attname = 'points'`)).toBe("bigint");
+    expect(await columnTypes(SUMMARY_VIEW)).toEqual({ id: "bigint", scored: "bigint" });
+    expect((await plan(after.expected)).changes).toEqual([]);
+  }, 60_000);
+
+  // Postgres retypes a column no view reads with the view standing, so the view stays.
+  test("an unchanged body in a plan that retypes a column it does not read is left standing", async () => {
+    await migrate((await model(scoredView(OVER_BADGES), SUMMARY_VIEW, [], BEFORE.others)).expected);
+    const oid = await viewOid(SUMMARY_VIEW);
+    const after = await model(scoredView(OVER_BADGES), SUMMARY_VIEW, [], [badge("long")]);
+
+    const changed = await plan(after.expected);
+    expect(kindsOf(changed.changes)).toEqual(["change-column-type"]);
+    await apply(changed.changes);
+    expect(await viewOid(SUMMARY_VIEW)).toBe(oid);
+    expect((await plan(after.expected)).changes).toEqual([]);
+  }, 60_000);
+
   // A derived body lists every table it reads, so a change to another table cannot reach it.
   test("a derived view that changes beside the same table change is replaced in place", async () => {
     const flag = (field: string) => summary(computed("flag", "boolean", { op: "isNull", arg: { field } }));

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

@@ -1237,13 +1237,18 @@ function recreateViewsDependingOnChangedTables(
       return viewReadsAnyColumn(sql, cols, readsEverything);
     });
     const prior = actualById.get(id);
-    // Where a triggering change rebuilds the table, the view in the database is stranded by
-    // every table ITS body reads, and that body may read one `dependsOn` leaves out: a
-    // table the new body no longer reads, or one an `@sql` body joins (ADR-0043). On
-    // Postgres the same case is settled when the view is planned (pushViewUpdate).
-    if (!columnScoped && prior !== undefined) {
-      for (const t of alteredTables.keys()) {
-        if (!tracked.includes(t) && liveViewReadsTable(prior, liveName(t))) tables.push(t);
+    // The view in the database is stranded by every table ITS body reads, and that body
+    // may read one `dependsOn` leaves out: a table the new body no longer reads, or one an
+    // `@sql` body joins (ADR-0043). Its own text says which. On Postgres it is stranded
+    // only when it also reads an altered column, as above. (A Postgres view whose
+    // definition changes never gets this far as a replace: pushViewUpdate plans the pair.)
+    if (prior !== undefined) {
+      const priorReadsEverything = prior.sql !== undefined && viewReadsEveryColumn(prior.sql);
+      for (const [t, cols] of alteredTables) {
+        if (tracked.includes(t) || !liveViewReadsTable(prior, liveName(t))) continue;
+        if (cols === null || prior.sql === undefined || viewReadsAnyColumn(prior.sql, cols, priorReadsEverything)) {
+          tables.push(t);
+        }
       }
     }
     if (tables.length === 0) continue;

server/typescript/packages/migrate-ts/test/unit/diff-view-recreate-columns.test.ts

@@ -118,3 +118,39 @@ describe("D4 — a view is recreated only when the migration alters a column it
     expect(kinds(r)).not.toContain("replace-view");
   });
 });
+
+// A view's `dependsOn` lists the tables the model knows it reads. An `@sql` body may join
+// another one (ADR-0043), and Postgres refuses `ALTER COLUMN … TYPE` on a column that a
+// standing view reads, whichever table holds it
+// (https://www.postgresql.org/docs/current/sql-altertable.html, SET DATA TYPE). The view's
+// stored body is what names that table.
+describe("an unchanged view whose stored body reads a table outside dependsOn (postgres)", () => {
+  const POINTS: ColumnDescriptor = { name: "points", sqlType: { kind: "integer", bits: 32 }, nullable: false };
+  const BONUS: ColumnDescriptor = { name: "bonus", sqlType: { kind: "integer", bits: 32 }, nullable: false };
+  const wide = (c: ColumnDescriptor): ColumnDescriptor => ({ ...c, sqlType: { kind: "integer", bits: 64 } });
+  const scores = (cols: ColumnDescriptor[]): TableDescriptor =>
+    ({ name: "scores", columns: [ID, ...cols], indexes: [], foreignKeys: [], primaryKey: ["id"], checks: [] });
+  const JOINED = "SELECT g.id AS id, (SELECT count(*) FROM scores s WHERE s.points > 0) AS scored FROM games g";
+  const joined = view({ sql: JOINED, fingerprint: viewFingerprint(JOINED) });
+  const run = (v: ViewDescriptor, expected: ColumnDescriptor[]) =>
+    diff(
+      { tables: [table([ID, LABEL]), scores(expected)], views: [v] },
+      { tables: [table([ID, LABEL]), scores([POINTS, BONUS])], views: [v] },
+      { dialect: "postgres", allow: { typeChange: true } },
+    );
+
+  test("a type change to a column the body reads recreates it, and names the table", async () => {
+    const r = await run(joined, [wide(POINTS), BONUS]);
+    expect(kinds(r)).toEqual(["change-column-type", "drop-view", "create-view"]);
+    expect(r.changes[1]).toMatchObject({ view: "v_games", reason: { kind: "unchanged", tables: ["scores"] } });
+    expect(r.blocked).toEqual([]);
+  });
+
+  test("a type change to a column of that table the body does not read leaves it alone", async () => {
+    expect(kinds(await run(joined, [POINTS, wide(BONUS)]))).toEqual(["change-column-type"]);
+  });
+
+  test("a view whose body does not name that table is left alone", async () => {
+    expect(kinds(await run(view(), [wide(POINTS), BONUS]))).toEqual(["change-column-type"]);
+  });
+});