Repository navigation
Commit 74fcec1
fix(driver-sql)!: PG introspectSchema reads each table's columns in its own schema and refuses a same-named pair across schemas (#24920)
Fixes #24707
Clause-②: no (narrowing)
Dev session `session_013timUNNCthRfQkhy9Hz2fV`, dispatched by the
`domain:engine` seat 1 (`session_01JtSQQ3EWsbNPzyYMug1tXy`) under claim
6292170810. The review of record is owed at `CONTRACT_REVIEW_TIER`, on
the narrowing face.
## What changes
`SqlDriver.introspectSchema`, PostgreSQL branch only
(`packages/drivers/driver-sql/src/sql-driver.ts`):
- **Columns read in the table's own schema.** The table listing now also
selects `table_schema` (and its position in `current_schemas(false)`).
Each table's columns are read with knex
`withSchema(schema).columnInfo()`, and the declared-order read
`introspectColumnOrder` uses `table_schema = ?` for that schema. Before,
both read by bare name, which knex scopes to `current_schema()`, the
first schema on the path only. `introspectColumns` /
`introspectColumnOrder` gain an optional trailing `schema` parameter.
Both are `protected`, so no new public member. Every other caller passes
none, and its SQL is byte-identical to before.
- **A same-named pair is refused, never dropped.** When the session's
`search_path` reaches one bare table name in more than one schema,
`introspectSchema` throws `RESOURCE_CONFLICT` / `status: 409`, naming
each colliding table with its schemas in `search_path` order. The
message gives the remedy: one schema per connection (the datasource's
`schema` setting sets its `search_path`), or a rename.
- **Unchanged:** a single-schema connection (the default `public`
deployment), and the SQLite and MySQL branches. The listing query gains
no `ORDER BY`, so a single-schema answer keeps its key order.
Not touched, per the claim: no `schema` member on `IntrospectedTable`,
no `packages/spec` change, no edit to `service-datasource` (read only,
as the consumer).
## Measured before and after (throwaway PostgreSQL 16.15 cluster, local
`postgres` binary)
The card's database: `public.customers`, `sales.orders`,
`sales.customers`, and `ALTER DATABASE … SET search_path = public,
sales`. It was read through `SqlDriver.introspectSchema` and through
`ExternalDatasourceService.listRemoteTables`, the service class
constructed with the driver as its `introspect`, with no server.
| database | before (`fcb40ad673`) | after (`1eeab8089`) |
|---|---|---|
| card (`public, sales`, `customers` in both) | tables `customers`
(public's 3 columns), `orders` with `columns: []` and `primaryKeys:
['id']`; `sales.customers` absent; `listRemoteTables` → `customers`
columnCount 3, `orders` columnCount 0 | `RESOURCE_CONFLICT` / 409:
`Colliding: 'customers' ('public', 'sales')`, from both
`introspectSchema` and `listRemoteTables` |
| card without `sales.customers` | (same zero-column `orders`) |
`orders` reads its 4 columns (`id`, `customer_id`, `total`,
`placed_at`); `listRemoteTables` → columnCount 3 and 4 |
| single schema (`public` only, `customers` + `orders` with an FK) |
sha256 of the JSON answer (with `introspectedAt` masked) `a273d463…f8` |
the same `a273d463…f8`: byte-identical |
## The collision answer: refusal (four axes)
Callers of `introspectSchema`: `ExternalDatasourceService`
(`listRemoteTables`, `generateObjectDraft`, `refreshCatalog`,
`validateObject*`, and `testConnection` for a non-primary datasource),
through `ObjectQL.introspectDatasource` or the driver directly
(`service-datasource/src/plugin.ts`). Also the datasource admin plugin's
connection test, only as a last fallback after `ping` / `checkHealth`,
which `SqlDriver` has. `driver-turso` overrides it (SQLite). No
migration differ or `os` command calls it
(`packages/cli/src/utils/unmanaged-tables.ts` mirrors its listing
statement and does not call it). Every caller already surfaces a throw:
the listing, draft and catalog doors return the error, and
`testConnection` answers `{ ok: false, error }`.
- **Business need:** the only connection that can collide is an external
PostgreSQL datasource whose `search_path` names two or more schemas
holding a same-named table, which is the finder's measured shape. A
single-schema connection can never collide, so no internal caller that
sees one schema is refused.
- **Long-term soundness:** "keep the first, name the other" has no
channel to name it in without a spec change (`IntrospectedSchema`
carries no warnings, and a `schema` member on `IntrospectedTable` is the
spec decision the triage withheld). That leaves a server log line, a
workaround in all but name.
- **Making AI mistakes hard:** a log line is invisible to the HTTP / AI
caller, so for them "keep the first" is the same silent drop this card
exists to end. A loud refusal with a named remedy cannot be mistaken for
a complete listing.
- **Startup focus:** no new surface, no new option, no spec key, and no
new gate.
Rejected third shape, recorded: keying the shadowed twin as a qualified
`sales.customers`. The consumers' `parseQualified` would read it, but a
key whose spelling depends on context is exactly what this file's
`IntrospectedForeignKey.referencedTable` note rejects (#11377), and it
is outside the two options the claim names.
## Pins
New
`packages/drivers/driver-sql/src/sql-driver-24707-introspect-pg-multischema.test.ts`,
a live-PG cell declared through `declareDialectCell` (a named skip
without `OS_TEST_POSTGRES_URL`). In CI it runs in `Temporal Conformance
(live PG + MySQL)`'s driver-sql step (`ci.yml`, "Run driver-sql suite
against both live servers"), which is in `main`'s full battery. In PR
runs the file is a named skip.
1. non-vacuity: the session's path is both schemas, and the bare-name
`columnInfo()` reads zero columns for the later schema's table;
2. the later schema's table reads its own columns (exact array) and its
key;
3. a same-named pair is refused: `{ code: 'RESOURCE_CONFLICT', status:
409 }`, and the message names the table and both schemas;
4. control: a single-schema session lists only its own table, and the
schema-scoped column read equals the bare-name read.
Ablation (committed fix, `node scripts/ablation-replace.mjs`, one leg
per mutation, each restored blob-equal to `HEAD` `74cd151dd2be` with
`git diff HEAD` empty; the test imports `../src`, so no dist leg):
- leg A, the column read reverted to the bare name: pin 2 red (1 failed
/ 3 passed);
- leg B, the refusal deleted: pin 3 red (`expected undefined to be
defined`), 1 failed / 3 passed.
Pin sweep (`introspectSchema`, `introspectColumns`, `columnInfo`,
`current_schemas`, `columnCount` across `packages/`, `qa/`,
`docs/qa/platform-checklist/`, `content/docs/`): no existing pin moves.
`unmanaged-tables.test.ts` asserts `current_schemas(false)` in the CLI's
own mirror statement, which is unchanged. The checklist's
external-datasource items run on a SQLite fixture, and their knownGaps
text about bare names from `current_schemas(false)` stays true. No
`docs/qa/platform-checklist/**` edit.
## Verification record (head `1eeab8089`, after merging `origin/main`
`a8b44d407`)
- `pnpm --filter @objectstack/driver-sql typecheck`: exit 0 (its
tsconfig includes `src/**/*`, so the new test is type-checked).
- `pnpm --filter @objectstack/driver-sql test`: exit 0, 224 files passed
/ 12 skipped, 3687 tests passed / 209 skipped (no live URL, as in `Test
Core`). It ran on `b61cf5026`; the merge brought only `platform-objects`
files.
- The introspection live files (21 files) with `OS_TEST_POSTGRES_URL` on
the local cluster: exit 0, 170 passed / 10 skipped (MySQL cells, no
MySQL URL).
- The new pin file on the merged head, live PG: exit 0, 4 passed.
- `pnpm --filter @objectstack/service-datasource test` (the consumer),
after building its dependency closure: exit 0, 48 files / 862 tests
passed.
- `pnpm exec turbo run build --concurrency=2` on the merged head: exit
0, 77/77 tasks.
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`: 70 families, each run with its exit code
captured before any pipe, all 0. `check:dual-build-cjs-loads` first
answered 3 (PREREQUISITE NOT MET, no dist) and was re-run after the full
build: exit 0. `check:type-check-debt` first hit my per-command 280 s
cap (124) while the box was busy and was re-run: exit 0. `--ran`
reconciliation: 70 derived, 70 run, 0 NOT-MEASURED, all 70 carrying an
exit code.
- `pnpm --filter @objectstack/core test:repo`: exit 0, 6 files / 82
tests passed.
## Acceptance notes
- The three key reads `introspectSchema` makes after the columns
(`introspectPrimaryKeys` by `?::regclass`, `introspectForeignKeys` and
`introspectUniqueConstraints` scoped to `current_schemas(false)`) are
unchanged. With base-table collisions refused, each now answers for the
one base table of that name on the path. One residual shape is a
read-only inference, not measured: a VIEW (or other relation) named like
a later schema's table but sitting in an earlier schema would still
capture the `regclass` resolution of the primary-key read. Noted, not
filed.
- The CLI's `unmanaged-tables` sweep lists names across the same schemas
and is untouched.
---
_Generated by [Claude
Code](https://claude.ai/code/session_013timUNNCthRfQkhy9Hz2fV)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent d606fc1 commit 74fcec1
3 files changed
Lines changed: 271 additions & 11 deletions
File tree
- .changeset
- packages/drivers/driver-sql/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
0 commit comments