diff --git a/.changeset/24707-pg-multischema-introspect.md b/.changeset/24707-pg-multischema-introspect.md new file mode 100644 index 00000000000..a2b25affdd5 --- /dev/null +++ b/.changeset/24707-pg-multischema-introspect.md @@ -0,0 +1,19 @@ +--- +'@objectstack/driver-sql': minor +--- + +fix(driver-sql)!: on PostgreSQL, `introspectSchema` reads each table's columns in the table's own schema, and refuses a `search_path` that reaches one table name in two schemas instead of silently dropping one + +`introspectSchema` lists the base tables of every schema in `current_schemas(false)` and keys the answer by bare table name. Two things went wrong when a connection's `search_path` named more than one schema (`ALTER DATABASE … SET search_path = public, sales`). First, each table's columns came from knex `columnInfo()` on the bare name, which reads only the first schema on the path, so a table that lives only in a later schema (`sales.orders`) was listed with no columns. `listRemoteTables` answered it `columnCount: 0`, and a draft built from it had no fields. Second, a same-named table in a later schema (`sales.customers` behind `public.customers`) was dropped without a word, because one key cannot hold two tables. The PostgreSQL branch now reads every table's columns in its own schema. When one bare name is reached in more than one schema, it refuses the whole introspection with `RESOURCE_CONFLICT` / `409`, naming each colliding table with its schemas in `search_path` order. A connection that reaches one schema, the default `public` deployment included, answers exactly what it answered before. The SQLite and MySQL branches are unchanged. + +Clause-②: no (narrowing) + + + +**BREAKING** (introspection's accept set narrows on PostgreSQL), shipped as `minor` under the launch-window convention for breaking changes. + +- **FROM:** a PostgreSQL connection whose `search_path` reached a table name in two schemas got a successful `introspectSchema()` answer holding one of the two tables (the one in the earliest schema on the path), and so did every caller built on it: `listRemoteTables`, object drafting, catalog refresh and the external datasource's connection test. +- **TO:** the same call throws `RESOURCE_CONFLICT` (`status: 409`), naming each colliding table and its schemas, for example `'customers' ('public', 'sales')`. Those callers surface that error. +- **Also changed (a wrong answer made right, no accept set moved):** a table that lives only in a later schema on the path now reports its own columns where it reported none. +- **Unchanged:** a connection that reaches one schema, the default `public` deployment included. One schema cannot hold two same-named tables, so it is never refused, and its answer is byte-identical to before. +- **Fix:** point the connection at one schema (the datasource's `schema` setting sets its `search_path`), or rename one of the two tables on the remote database. diff --git a/packages/drivers/driver-sql/src/sql-driver-24707-introspect-pg-multischema.test.ts b/packages/drivers/driver-sql/src/sql-driver-24707-introspect-pg-multischema.test.ts new file mode 100644 index 00000000000..866d474cf38 --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-24707-introspect-pg-multischema.test.ts @@ -0,0 +1,159 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24707] `introspectSchema` on a PostgreSQL session whose `search_path` + * names MORE THAN ONE schema. + * + * The table listing spans every `current_schemas(false)` schema, but the + * answer is keyed by BARE name, and each table's columns were read through + * knex `columnInfo()` on that bare name — which scopes to `current_schema()`, + * the FIRST schema on the path only. Two wrong answers followed, both measured + * on PostgreSQL 16 with `search_path = public, sales`: + * + * 1. a table living only in the later schema (`sales.orders`) was listed with + * `columns: []` — `listRemoteTables` answered it `columnCount: 0`, and a + * draft built from it would carry no fields; + * 2. a same-named table in the later schema (`sales.customers` behind + * `public.customers`) was silently dropped: one key, two tables. + * + * The fix reads each table's columns in its own schema, and refuses the + * listing — `RESOURCE_CONFLICT` / 409, naming the table and both schemas — + * when one bare key would have to hold two tables. The third case is the + * control: a session that reaches ONE schema answers exactly what the + * bare-name read always answered. + * + * ## Why this file is PG-only + * + * SQLite has no schemas and the MySQL arm lists `DATABASE()` alone, so neither + * can put two schemas on one session's resolution path. The cell list is + * exactly `pg`, declared through `declareDialectCell` so an unprovisioned run + * is a NAMED skip, never a silent pass. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { SqlDriver } from '../src/index.js'; +import { + PG_CELL, + currentLiveSchema, + declareDialectCell, + type DialectCell, +} from './live-dialect-matrix.testkit.js'; + +const MATRIX = 'introspectSchema multi-schema search_path'; + +/** Lives in this file's own schema — the FIRST on the path. */ +const CUSTOMERS = 'os24707_customers'; +/** Lives only in the SECOND schema on the path. */ +const ORDERS = 'os24707_orders'; + +/** `introspectColumns` is `protected`; this is the narrowest way to reach it. */ +class ColumnProbeDriver extends SqlDriver { + columns(table: string, schema?: string) { + return this.introspectColumns(table, schema); + } +} + +function declareMultiSchemaSuite(cell: DialectCell): void { + describe(`introspectSchema multi-schema search_path — ${cell.label} (#24707)`, () => { + /** Session whose `search_path` is `here, there`. */ + let driver: ColumnProbeDriver; + /** Session whose `search_path` is `here` alone — the control. */ + let single: ColumnProbeDriver; + let here: string; + let there: string; + + beforeAll(async () => { + here = currentLiveSchema(); + there = `${here}_b`; + // Postgres TRUNCATES an over-long identifier silently, which would fold + // the second schema back onto this file's own. + expect(there.length, `${there} exceeds Postgres' 63-byte identifier limit`).toBeLessThanOrEqual(63); + + single = new ColumnProbeDriver(cell.config()); + driver = new ColumnProbeDriver({ ...cell.config(), searchPath: [here, there] }); + + await single.execute(`drop schema if exists "${there}" cascade`); + await single.execute(`create schema "${there}"`); + await single.execute(`drop table if exists "${here}".${CUSTOMERS} cascade`); + await single.execute( + `create table "${here}".${CUSTOMERS} (id serial primary key, name text not null, email varchar(120) unique)`, + ); + await single.execute( + `create table "${there}".${ORDERS} (id serial primary key, customer_id integer, total numeric(10, 2), placed_at timestamptz)`, + ); + }); + + afterAll(async () => { + await single.execute(`drop schema if exists "${there}" cascade`).catch(() => {}); + await single.execute(`drop table if exists "${here}".${CUSTOMERS} cascade`).catch(() => {}); + await driver.disconnect().catch(() => {}); + await single.disconnect().catch(() => {}); + }); + + it('the fixture really reaches two schemas, and a bare-name read sees only the first', async () => { + // Non-vacuity: the session's path is both schemas, in this order… + const path: any = await driver.execute( + `select s::text as s from unnest(current_schemas(false)) with ordinality as t(s, ord) order by ord`, + ); + expect(path.rows.map((r: any) => r.s)).toEqual([here, there]); + // …and the pre-fix read — `columnInfo()` on the bare name — answers + // nothing for the later schema's table. If this ever reads columns, the + // next case is measuring nothing. + expect(Object.keys(await driver.getKnex()(ORDERS).columnInfo())).toEqual([]); + }); + + it("reads a later schema's table in its own schema, not as zero columns", async () => { + const schema = await driver.introspectSchema(); + + expect(Object.keys(schema.tables).sort()).toEqual([CUSTOMERS, ORDERS]); + expect(schema.tables[ORDERS].columns.map((c) => [c.name, c.type, c.primaryKey])).toEqual([ + ['id', 'integer', true], + ['customer_id', 'integer', false], + ['total', 'numeric', false], + ['placed_at', 'timestamp with time zone', false], + ]); + expect(schema.tables[ORDERS].primaryKeys).toEqual(['id']); + // The first schema's table is unaffected by the second being on the path. + expect(schema.tables[CUSTOMERS].columns.map((c) => c.name)).toEqual(['id', 'name', 'email']); + }); + + it('refuses a same-named pair across schemas instead of dropping one', async () => { + await single.execute(`create table "${there}".${CUSTOMERS} (code varchar(20) primary key, region text)`); + try { + let thrown: any; + await driver.introspectSchema().catch((e) => { + thrown = e; + }); + expect(thrown, 'a same-named pair must not be answered with one of the two tables').toBeDefined(); + expect({ code: thrown.code, status: thrown.status }).toEqual({ code: 'RESOURCE_CONFLICT', status: 409 }); + // The named subjects: the colliding name and both schemas, first on + // the path first — what the operator needs to act on. + expect(thrown.message).toContain(`'${CUSTOMERS}' ('${here}', '${there}')`); + } finally { + await single.execute(`drop table if exists "${there}".${CUSTOMERS} cascade`); + } + }); + + it('a session that reaches ONE schema answers exactly what the bare-name read does', async () => { + const schema = await single.introspectSchema(); + + // The second schema is not on this session's path, so it lists nothing. + expect(Object.keys(schema.tables)).toEqual([CUSTOMERS]); + // And the schema-scoped column read equals the bare-name one on the + // first schema — the single-schema answer the fix must not move. + // (`introspectSchema` then flips `primaryKey` / `isUnique` from the key + // reads, so the comparison is of the read itself.) + expect(await single.columns(CUSTOMERS, here)).toEqual(await single.columns(CUSTOMERS)); + const unflagged = (cols: { primaryKey?: boolean; isUnique?: boolean }[]) => + cols.map(({ primaryKey: _pk, isUnique: _uq, ...rest }) => rest); + expect(unflagged(schema.tables[CUSTOMERS].columns)).toEqual(unflagged(await single.columns(CUSTOMERS))); + expect(schema.tables[CUSTOMERS].columns.map((c) => [c.name, c.primaryKey, c.isUnique])).toEqual([ + ['id', true, false], + ['name', false, false], + ['email', false, true], + ]); + }); + }); +} + +declareDialectCell(PG_CELL, MATRIX, declareMultiSchemaSuite); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 206bca5f0bc..74cd151dd2b 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -4858,6 +4858,42 @@ function classifyFilterKey( return 'clause'; } +/** + * [#24707] PostgreSQL only: the session's `search_path` reaches one table name + * in more than one schema, and {@link SqlDriver.introspectSchema} keys its + * answer by bare name — one key cannot hold two tables. + * + * This used to be silent: the later table was overwritten by the earlier one + * (`sales.customers` vanished behind `public.customers`), and nothing in the + * answer said a table was missing. Keeping the first and logging the second + * is not loud either — the HTTP and AI callers of the listing never see a + * server log, so to them it is the same silent drop. The answer is a refusal + * naming every colliding name with its schemas in `search_path` order, and + * the remedy: one schema per connection (the datasource's `schema` setting is + * what sets its `search_path`), or a rename. A connection that reaches one + * schema — the default `public` deployment — can never collide, so no caller + * that sees one schema is refused. + * + * `collisions` is non-empty; each entry's `schemas` has two or more members. + */ +function postgresTableNameCollisionError(collisions: ReadonlyArray<{ table: string; schemas: string[] }>): Error { + const list = (names: string[]) => names.map((s) => `'${s}'`).join(', '); + const [first] = collisions; + const err = new Error( + `Cannot introspect this PostgreSQL connection: its search_path reaches ${collisions.length} table name(s) ` + + `in more than one schema, for example '${first.table}' in ${list(first.schemas)}. ` + + `Introspection keys each table by its bare name, so one key cannot hold both, and the bare name ` + + `resolves only to '${first.schemas[0]}.${first.table}'; listing it once would hide the other ` + + `table and its columns. Colliding: ` + + collisions.map((c) => `'${c.table}' (${list(c.schemas)})`).join('; ') + + `. Point the connection at one schema (the datasource's 'schema' setting sets its search_path), ` + + `or rename one of the tables.`, + ) as Error & { code?: string; status?: number }; + err.code = StandardErrorCode.enum.RESOURCE_CONFLICT; + err.status = 409; + return err; +} + // ── Introspection Types ────────────────────────────────────────────────────── /** @@ -15454,19 +15490,50 @@ export class SqlDriver implements IDataDriver { // moment later than the first table it actually read. const introspectedAt = new Date().toISOString(); let tableNames: string[] = []; + // PostgreSQL only (#24707): the schema each listed table lives in, so its + // columns are read THERE. Every other dialect leaves it empty. + const schemaByTable = new Map(); if (this.isPostgres) { // Every schema the session can actually reach unqualified, rather than // the literal `'public'` — same answer for a default deployment, where // `current_schemas(false)` is exactly `{public}`, and the right answer for // a session pointed at another schema, where the literal listed nothing. + // + // `table_schema` and its `search_path` position are selected too + // (#24707). The result is keyed by BARE name, so two schemas holding the + // same name used to collapse into one key with the later table silently + // dropped — refused below instead. And the schema is what the column read + // needs: knex `columnInfo()` on a bare name reads `current_schema()`, the + // FIRST schema on the path only, so a table in a later schema reported + // zero columns. No `ORDER BY` is added: the key order of a single-schema + // answer stays exactly what it was. const result = await this.knex.raw(` - SELECT table_name + SELECT table_schema, table_name, + array_position(current_schemas(false), table_schema::name) AS path_position FROM information_schema.tables WHERE table_schema = ANY (current_schemas(false)) AND table_type = 'BASE TABLE' `); - tableNames = result.rows.map((row: any) => row.table_name); + const rows = result.rows as { table_schema: string; table_name: string; path_position: number }[]; + const schemasByTable = new Map(); + for (const row of rows) { + const seen = schemasByTable.get(row.table_name); + const entry = { schema: row.table_schema, position: Number(row.path_position) }; + if (seen) seen.push(entry); + else schemasByTable.set(row.table_name, [entry]); + } + const collisions = [...schemasByTable] + .filter(([, schemas]) => schemas.length > 1) + .map(([table, schemas]) => ({ + table, + schemas: schemas.sort((a, b) => a.position - b.position).map((s) => s.schema), + })) + .sort((a, b) => (a.table < b.table ? -1 : a.table > b.table ? 1 : 0)); + if (collisions.length > 0) throw postgresTableNameCollisionError(collisions); + + tableNames = rows.map((row) => row.table_name); + for (const row of rows) schemaByTable.set(row.table_name, row.table_schema); } else if (this.isMysql) { const result = await this.knex.raw(` SELECT table_name @@ -15493,7 +15560,11 @@ export class SqlDriver implements IDataDriver { // read as absent (#11161). Every in-tree caller of `introspectSchema` // already handles a throw (the datasource health check reports // `{ ok: false }`, the REST/CLI seams surface the error). - const columns = await this.introspectColumns(tableName); + // On PostgreSQL the columns are read in the table's OWN schema (#24707); + // the three key reads below already scope to `current_schemas(false)` + // or resolve along `search_path`, which the refusal above makes + // unambiguous among base tables. + const columns = await this.introspectColumns(tableName, schemaByTable.get(tableName)); const foreignKeys = await this.introspectForeignKeys(tableName); const primaryKeys = await this.introspectPrimaryKeys(tableName); const uniqueConstraints = await this.introspectUniqueConstraints(tableName); @@ -20866,19 +20937,21 @@ export class SqlDriver implements IDataDriver { * catalog's ordinal rather than trusting a plan. * * Each arm reads the same catalog `columnInfo()` populates, with the same - * scoping knex itself applies there (PG: `current_schema()`; MySQL: - * `DATABASE()`; SQLite: `PRAGMA table_info`, whose `cid` is the ordinal). + * scoping knex itself applies there (PG: `current_schema()`, or the named + * `schema` exactly as knex's `withSchema()` scopes `columnInfo()` — #24707; + * MySQL: `DATABASE()`; SQLite: `PRAGMA table_info`, whose `cid` is the + * ordinal). */ - protected async introspectColumnOrder(tableName: string): Promise { + protected async introspectColumnOrder(tableName: string, schema?: string): Promise { if (this.isPostgres) { const result = await this.knex.raw( `SELECT column_name FROM information_schema.columns WHERE table_name = ? AND table_catalog = current_database() - AND table_schema = current_schema() + AND table_schema = ${schema === undefined ? 'current_schema()' : '?'} ORDER BY ordinal_position`, - [tableName], + schema === undefined ? [tableName] : [tableName, schema], ); return result.rows.map((row: any) => row.column_name); } @@ -20904,8 +20977,17 @@ export class SqlDriver implements IDataDriver { return []; } - protected async introspectColumns(tableName: string): Promise { - const columnInfo = (await this.knex(tableName).columnInfo()) as Record; + /** + * `schema` (PostgreSQL, #24707) reads the table in that schema rather than + * in `current_schema()`. Omitted, the read is the bare-name one it always + * was — the first schema on `search_path` — which is right for every caller + * that names a table this driver manages. {@link introspectSchema} lists + * tables across every schema on the path, so it names each table's own. + */ + protected async introspectColumns(tableName: string, schema?: string): Promise { + let catalog = this.knex(tableName); + if (schema !== undefined) catalog = catalog.withSchema(schema); + const columnInfo = (await catalog.columnInfo()) as Record; const columns: IntrospectedColumn[] = []; // #11163: emit in DECLARED order, not the `columnInfo()` object's @@ -20918,7 +21000,7 @@ export class SqlDriver implements IDataDriver { // in `columnInfo()` order rather than dropped, so the merge can reorder // but never lose a column. const has = (name: string) => Object.prototype.hasOwnProperty.call(columnInfo, name); - const orderedNames = (await this.introspectColumnOrder(tableName)).filter(has); + const orderedNames = (await this.introspectColumnOrder(tableName, schema)).filter(has); const seen = new Set(orderedNames); for (const name of Object.keys(columnInfo)) { if (!seen.has(name)) orderedNames.push(name);