fix(nextly): introspection resolves a table the way an unqualified write does

.changeset/pg-introspection-resolves-like-the-writer.md

@@ -0,0 +1,43 @@
+---
+"@nextlyhq/adapter-drizzle": patch
+"@nextlyhq/adapter-mysql": patch
+"@nextlyhq/adapter-postgres": patch
+"@nextlyhq/adapter-sqlite": patch
+"@nextlyhq/admin": patch
+"@nextlyhq/admin-css": patch
+"@nextlyhq/blocks-engine": patch
+"@nextlyhq/blocks-react": patch
+"@nextlyhq/builder": patch
+"create-nextly-app": patch
+"@nextlyhq/eslint-config": patch
+"@nextlyhq/eslint-plugin": patch
+"@nextlyhq/module-specifiers": patch
+"nextly": patch
+"@nextlyhq/plugin-form-builder": patch
+"@nextlyhq/plugin-page-builder": patch
+"@nextlyhq/plugin-sdk": patch
+"@nextlyhq/plugin-seo": patch
+"@nextlyhq/prettier-config": patch
+"@nextlyhq/storage-s3": patch
+"@nextlyhq/storage-uploadthing": patch
+"@nextlyhq/storage-vercel-blob": patch
+"@nextlyhq/telemetry": patch
+"@nextlyhq/tsconfig": patch
+"@nextlyhq/ui": patch
+---
+
+On PostgreSQL, Nextly reads a database's current shape to work out what a
+migration should change. Those reads looked in a schema called `public`, while
+every statement Nextly writes is unqualified and lands wherever the connection's
+`search_path` points. On a deployment that uses its own schema — one per tenant,
+or just a house convention — the two disagreed.
+
+The reads now resolve a table the same way the writes do, quoting the name first
+so one whose spelling carries capitals — which a custom table name may — resolves
+to itself rather than to nothing. Nothing changes for a database that uses
+`public`, which is the default.
+
+What it fixes on the others: columns that exist read as absent, so a migration
+offered to add what was already there; and where a table of the same name existed
+in `public`, its shape answered for the real one — comparing against a table
+nothing writes to.

packages/nextly/src/domains/schema/pipeline/diff/__tests__/introspect-search-path.integration.test.ts

@@ -0,0 +1,140 @@
+/**
+ * Introspection must read the table the WRITES hit, not one named `public`.
+ *
+ * Every statement this package emits is unqualified — the DDL that created the
+ * tables and the DDL the diff generates — so PostgreSQL resolves it across the
+ * whole `search_path`. A read filtered on a schema NAME asks a different
+ * question, and the answers diverge exactly where it matters: a deployment whose
+ * search path is `tenant, public`.
+ *
+ * Only a real database can establish this. The predicate is SQL the server
+ * evaluates, and every failure mode is silent — columns that exist read as
+ * absent, so the diff proposes to add what is already there; or a same-named
+ * table in another schema answers for this one.
+ *
+ * 🔴 A DECOY, not an empty schema. Pointing the search path somewhere empty
+ * would prove only that something was found somewhere; a test that cannot tell
+ * "read the right table" from "read any table" passes on a predicate that names
+ * `public` outright.
+ *
+ * 🔴 ONE SESSION, held open for the whole file. `PostgresAdapter.getDrizzle()`
+ * wraps a `pg.Pool`, so `SET search_path` binds to whichever client served that
+ * statement and the next `execute()` may run on another — which would make this
+ * file assert against the decoy at random. A `Client` is one connection by
+ * construction, so the search path this sets is the one every read below uses.
+ *
+ * @module domains/schema/pipeline/diff/__tests__/introspect-search-path.integration
+ */
+import { sql } from "drizzle-orm";
+import { drizzle } from "drizzle-orm/node-postgres";
+import { Client } from "pg";
+import { afterAll, beforeAll, describe, expect, it } from "vitest";
+
+import { queryLiveColumnTypes } from "../../live-column-types";
+import { introspectLiveSnapshot } from "../introspect-live";
+
+const URL = process.env.TEST_POSTGRES_URL ?? "";
+
+// Dialect gate, matching the other PostgreSQL integration tests in this package.
+const describePg = describe.skipIf(!URL);
+
+const TENANT = "nx_search_path_probe";
+const TABLE = "nx_search_path_subject";
+// Uppercase on purpose: `resolveCollectionTableName` passes an author's `dbName`
+// through verbatim, so a real table can carry capitals — and an unquoted name
+// handed to `to_regclass` is folded to lower case and resolves to nothing.
+const MIXED_CASE = "nx_SearchPath_Mixed";
+
+describePg("introspection follows the search path (postgres)", () => {
+  let client: Client | undefined;
+  // Typed from the call rather than from `typeof drizzle`, whose default
+  // overload infers a Pool-backed client and does not accept this one.
+  let db: ReturnType<typeof drizzle<Record<string, never>, Client>>;
+
+  beforeAll(async () => {
+    if (!URL) return;
+    client = new Client({ connectionString: URL });
+    await client.connect();
+    db = drizzle({ client });
+
+    const run = (text: string) => client!.query(text);
+
+    await run(`DROP TABLE IF EXISTS public."${TABLE}"`);
+    await run(`DROP SCHEMA IF EXISTS ${TENANT} CASCADE`);
+    await run(`CREATE SCHEMA ${TENANT}`);
+
+    // The decoy, in `public`: same name, a column the real one does not have.
+    // A read pinned to `public` finds THIS one and reports its shape.
+    await run(`CREATE TABLE public."${TABLE}" (decoy_only integer)`);
+    // The subject, in the tenant schema, carrying a different column.
+    await run(`CREATE TABLE ${TENANT}."${TABLE}" (subject_only text)`);
+    await run(`CREATE INDEX ix_subject ON ${TENANT}."${TABLE}" (subject_only)`);
+    // And one whose name survives only if it is quoted before it is resolved.
+    await run(`CREATE TABLE ${TENANT}."${MIXED_CASE}" (mixed_only text)`);
+
+    // What a deployment on a non-default search path looks like.
+    await run(`SET search_path TO ${TENANT}, public`);
+  });
+
+  afterAll(async () => {
+    if (client === undefined) return;
+    await client.query(`SET search_path TO public`);
+    await client.query(`DROP TABLE IF EXISTS public."${TABLE}"`);
+    await client.query(`DROP SCHEMA IF EXISTS ${TENANT} CASCADE`);
+    await client.end();
+  });
+
+  it("proves the session is the one that was configured", async () => {
+    // The control for every assertion below. If the search path were not the
+    // one set in `beforeAll` — a pooled connection, a reset — the decoy would
+    // answer and the failures would look like defects in the predicate.
+    const shown = await db.execute(sql`SHOW search_path`);
+    const rows = (shown as unknown as { rows: { search_path: string }[] }).rows;
+    expect(rows[0]?.search_path).toContain(TENANT);
+  });
+
+  it("reads the columns of the table an unqualified write would hit", async () => {
+    const live = await introspectLiveSnapshot(db, "postgresql", [TABLE]);
+    const columns = live.tables
+      .find(t => t.name === TABLE)
+      ?.columns.map(c => c.name);
+
+    expect(columns).toEqual(["subject_only"]);
+    // Named separately: the decoy's column appearing IS the defect, and saying
+    // so makes a failure readable rather than a diff of two arrays.
+    expect(columns).not.toContain("decoy_only");
+  });
+
+  it("reads the indexes of that table, not a same-named one elsewhere", async () => {
+    const live = await introspectLiveSnapshot(db, "postgresql", [TABLE]);
+    const indexes = live.tables.find(t => t.name === TABLE)?.indexes ?? [];
+
+    // The decoy carries no index, so a read pinned to `public` reports none —
+    // which reads as "this table has no indexes" rather than as an error.
+    expect(indexes.map(i => i.name)).toContain("ix_subject");
+  });
+
+  it("resolves a table whose name carries capitals", async () => {
+    // 🔴 `to_regclass` reparses its argument as an identifier reference, so an
+    // unquoted `nx_SearchPath_Mixed` folds to lower case and resolves to
+    // nothing — and the table reads as ABSENT, which makes a diff propose to
+    // create one that already exists.
+    const live = await introspectLiveSnapshot(db, "postgresql", [MIXED_CASE]);
+    const columns = live.tables
+      .find(t => t.name === MIXED_CASE)
+      ?.columns.map(c => c.name);
+
+    expect(columns).toEqual(["mixed_only"]);
+  });
+
+  it("reports live column TYPES from the same table", async () => {
+    // A second query with the same predicate, feeding the type diff. Pinned to
+    // `public` it answers for the decoy: a column the subject does not have,
+    // and none of the ones it does.
+    const types = await queryLiveColumnTypes(db, "postgresql", [TABLE]);
+    const forTable = types.get(TABLE);
+
+    expect(forTable?.get("subject_only")).toBe("text");
+    expect(forTable?.has("decoy_only")).toBe(false);
+  });
+});

packages/nextly/src/domains/schema/pipeline/diff/introspect-live.ts

@@ -17,6 +17,11 @@
 import type { SupportedDialect } from "@nextlyhq/adapter-drizzle/types";
 import { sql } from "drizzle-orm";
 
+import {
+  PG_CLASS_IS_THE_RELATION_THE_WRITES_HIT,
+  PG_RELATION_THE_WRITES_HIT,
+} from "../pg-visible-relation";
+
 import { sizeFromDeclaration } from "./declared-size";
 import type {
   ColumnSpec,
@@ -229,6 +234,10 @@ export async function introspectLiveSnapshot(
     // that is not exactly a `nextval()` call yields NULL from the substring
     // and so falls to false, which is the safe direction: a default the diff
     // does not recognise is reported, never swallowed.
+    // Scoped by `PG_RELATION_THE_WRITES_HIT`, which carries the reasoning: the
+    // read has to resolve the same relation an unqualified statement does, and
+    // neither a schema name nor `current_schema()` does that.
+    //
     // `is_primary_key` comes from `pg_index.indisprimary` rather than
     // `information_schema.table_constraints`, so it needs no second round trip
     // and reports the same key the index query deliberately excludes. A live
@@ -262,7 +271,7 @@ export async function introspectLiveSnapshot(
                    false
                  ) AS owned_sequence_default
           FROM information_schema.columns c
-          WHERE c.table_schema = 'public'
+          WHERE ${PG_RELATION_THE_WRITES_HIT}
             AND c.table_name IN (${tableNamesIn})
           ORDER BY c.table_name, c.ordinal_position`
     )) as { rows: PgRow[] };
@@ -271,21 +280,20 @@ export async function introspectLiveSnapshot(
     // (indisprimary) and partial indexes (indpred). Expression indexes yield no
     // pg_attribute row and are naturally excluded.
     //
-    // Scoped to `public` like the column query above. `pg_class.relname` is
-    // unique per schema, not per database, so without the namespace join a
-    // same-named table in another schema contributes its indexes to these rows
-    // and `attachIndexes` groups them under the same name — reporting indexes
-    // that are not on the table being introspected, and masking the absence of
-    // ones that should be.
+    // Scoped through the relation like the column query above. `pg_class.relname`
+    // is unique per schema, not per database, so without a scope a same-named
+    // table in another schema contributes its indexes to these rows and
+    // `attachIndexes` groups them under the same name — reporting indexes that
+    // are not on the table being introspected, and masking the absence of ones
+    // that should be.
     const idxResult = (await dbTyped.execute(
       sql`SELECT t.relname AS table, i.relname AS index, ix.indisunique AS unique,
                  a.attname AS column, array_position(ix.indkey, a.attnum) AS ord
           FROM pg_class t
-          JOIN pg_namespace n ON n.oid = t.relnamespace
           JOIN pg_index ix ON ix.indrelid = t.oid
           JOIN pg_class i ON i.oid = ix.indexrelid
           JOIN pg_attribute a ON a.attrelid = t.oid AND a.attnum = ANY(ix.indkey)
-          WHERE n.nspname = 'public'
+          WHERE ${PG_CLASS_IS_THE_RELATION_THE_WRITES_HIT}
             AND t.relname IN (${tableNamesIn})
             AND ix.indisprimary = false
             AND ix.indpred IS NULL

packages/nextly/src/domains/schema/pipeline/live-column-types.ts

@@ -22,6 +22,8 @@
 import type { SupportedDialect } from "@nextlyhq/adapter-drizzle/types";
 import { sql } from "drizzle-orm";
 
+import { PG_RELATION_THE_WRITES_HIT } from "./pg-visible-relation";
+
 interface PgRow {
   table_name: string;
   column_name: string;
@@ -81,11 +83,14 @@ export async function queryLiveColumnTypes(
     // object with a `.rows` array, NOT a flat row array. Verified
     // empirically against real PG - reading `result` directly as an
     // array iterates zero rows even when the query returns matches.
+    // Scoped by `PG_RELATION_THE_WRITES_HIT`. Here a wrong answer is a column's
+    // live type read from a same-named table in another schema — which is the
+    // input the type diff trusts.
     const result = (await dbTyped.execute(
-      sql`SELECT table_name, column_name, udt_name
-          FROM information_schema.columns
-          WHERE table_schema = 'public'
-            AND table_name IN (${tableNamesIn})`
+      sql`SELECT c.table_name, c.column_name, c.udt_name
+          FROM information_schema.columns c
+          WHERE ${PG_RELATION_THE_WRITES_HIT}
+            AND c.table_name IN (${tableNamesIn})`
     )) as { rows: PgRow[] };
     for (const row of result.rows) {
       let cols = out.get(row.table_name);

packages/nextly/src/domains/schema/pipeline/pg-visible-relation.ts

@@ -0,0 +1,64 @@
+/**
+ * Which PostgreSQL relation a read is about, decided the way the WRITES decide.
+ *
+ * Every statement this package emits is unqualified — the DDL that created a
+ * table and the DDL a diff generates — so PostgreSQL resolves it across the
+ * whole `search_path`. A read that filters on a schema NAME asks a different
+ * question, and the two answers separate exactly where it matters: a deployment
+ * whose search path is `tenant, public`.
+ *
+ * 🔴 `current_schema()` is not the answer either. It is only the FIRST entry of
+ * the search path, so a table in `public` reached from `tenant, public` is
+ * written by the ALTER and missed by the read. `live-table-facts.ts` settled
+ * this for the same reason and resolves through `to_regclass`; this is that rule
+ * for the reads that go through `information_schema` rather than through an OID.
+ *
+ * The failures it prevents are silent and point the wrong way. Columns that
+ * exist read as absent, so a diff offers to add what is already there. And where
+ * a same-named table exists in another schema on the path, ITS shape answers for
+ * the real one — so the diff compares against a table nothing writes to.
+ *
+ * ## Why the name is quoted before it is resolved
+ *
+ * 🔴 `to_regclass` takes TEXT and reparses it as an identifier reference, so an
+ * unquoted catalog name is folded and split exactly as if it had been typed:
+ * `dc_LegacyPosts` resolves as `dc_legacyposts`, and a name containing a dot
+ * resolves as `schema.table`. Both are reachable — `resolveCollectionTableName`
+ * passes an author's `dbName` through verbatim, prefixing `dc_` and nothing
+ * else — and both would make an existing table read as ABSENT, which is the
+ * worst answer this module can give: a diff that proposes to create a table that
+ * is already there.
+ *
+ * `quote_ident` is the inverse of that parse, so the text goes back in as the
+ * one identifier the catalog says it is.
+ *
+ * @module domains/schema/pipeline/pg-visible-relation
+ */
+import { sql, type SQL } from "drizzle-orm";
+
+/**
+ * True for the one `information_schema.columns` row set whose relation an
+ * unqualified statement would reach.
+ *
+ * 🔴 Written against the alias `c`, because a predicate over
+ * `information_schema` has to name that view's own columns and they cannot be
+ * passed as parameters. Every caller aliases the view `c`; there are two, both
+ * in this directory, and a caller that aliases it otherwise gets a SQL error
+ * rather than a wrong answer — which is the safe direction for a mistake that
+ * would otherwise change WHICH TABLE is reported.
+ *
+ * Compared by identity rather than by spelling: `format('%I.%I', …)::regclass`
+ * is this row's relation, `to_regclass(…)` is the one the search path resolves,
+ * and the equality asks whether they are the same relation. The same device the
+ * sequence-ownership check in `introspect-live.ts` already uses.
+ */
+export const PG_RELATION_THE_WRITES_HIT: SQL = sql`format('%I.%I', c.table_schema, c.table_name)::regclass = to_regclass(quote_ident(c.table_name))`;
+
+/**
+ * The same question asked of a `pg_class` row, which carries the OID directly.
+ *
+ * Spelled here rather than inline at the index query so the two reads cannot
+ * drift: they must agree about which relation they are describing, or the
+ * columns come from one table and the indexes from another.
+ */
+export const PG_CLASS_IS_THE_RELATION_THE_WRITES_HIT: SQL = sql`t.oid = to_regclass(quote_ident(t.relname))`;