Skip to content

fix(driver-sql): scope the Postgres introspectForeignKeys catalog read to the session's own schemas - #11325

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-11201-introspect-fk-schema-scope
Aug 23, 2026
Merged

fix(driver-sql): scope the Postgres introspectForeignKeys catalog read to the session's own schemas#11325
os-zhuang merged 1 commit into
mainfrom
claude/issue-11201-introspect-fk-schema-scope

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#11201

The Postgres arm of SqlDriver.introspectForeignKeys filtered information_schema.table_constraints on tc.constraint_type = 'FOREIGN KEY' AND tc.table_name = ? with no table_schema predicate at all. Those views span every schema the session has privilege on, independently of search_path, so a table name existing in more than one schema had all of their foreign keys merged into one answer.

That is a wrong answer rather than a missing one, and it is consumed as fact: introspectSchema hangs the result on the table it just listed, and from there it reaches federated-object codegen, the persisted external_catalog (ADR-0015) and schema-drift comparison.

The change

One predicate, plus the comment explaining it:

ANDtc.table_schema= ANY (current_schemas(false))

No interface shape changes, and the accepted input set is not widened — a same-named table in another schema simply stops contributing foreign keys it never should have contributed.

Sibling-arm comparison, since the point is ONE resolution rule across the family

I compared every Postgres introspection arm in sql-driver.ts before writing, rather than copying the predicate from the issue:

armhow it scopes the Postgres readagrees?
introspectSchema (table listing, ~10293)WHERE table_schema = ANY (current_schemas(false))yes — carries the pin
introspectUniqueConstraints (~13416)AND tc.table_schema = ANY (current_schemas(false))yes — carries the pin
introspectIndexes (~9965)WHERE ix.indrelid = to_regclass(?)consistent, other side
introspectPrimaryKeys (~13294)WHERE i.indrelid = ?::regclassconsistent, other side
introspectColumnOrder (~13049)AND table_schema = current_schema()deliberate, documented
introspectForeignKeys (~13151)nothing — this PR

The two arms that carry this predicate agree with each other exactly, so there was no winner to pick: same spelling, same ANY (...) form, and the same placement as the last predicate in the WHERE, after tc.table_name = ?. This PR matches both. The existing JOIN correlations on kcu.table_schema / ccu.table_schema are left untouched.

The two pg_index arms are not a disagreement — pg_index takes a relation, not a schema name, so they reach the same session scoping by resolving the name to an OID through regclass. introspectColumnOrder uses current_schema() on purpose and says so in its docblock: it mirrors the scoping knex itself applies in columnInfo(), which is the catalog read it exists to re-order.

MySQL arm: checked, not affected

The MySQL arm of this same method already pins TABLE_SCHEMA = DATABASE() and reads KEY_COLUMN_USAGE directly. The defect is not expressible there, so nothing was widened into this PR. SQLite has no schemas (PRAGMA foreign_key_list is relation-scoped by construction).

Test — measured on a live PostgreSQL 16.13

New pin: packages/drivers/driver-sql/src/sql-driver-11201-introspect-fk-schema-scope.test.ts, PG-only, declared through declareDialectCell(PG_CELL, ...) so an unprovisioned run is a named skip and a red under OS_EXPECT_LIVE_DIALECT_MATRIX=1 — never a silent pass. The required Temporal Conformance (live PG + MySQL) job runs this cell against its real postgres:16 service container.

The fixture builds the collision the repo's own live-PG isolation (#9350 — one schema per test file inside one database) already makes routine: two same-named os11201_orders tables in two schemas, each with a different foreign key to a differently-named parent, only one schema on search_path.

The interesting assertion is an absence, which goes green for free on a fixture that never collided — so the first case is a non-vacuity pin: it re-issues the pre-fix predicate verbatim and requires it to see both constraints, and separately confirms to_regclass resolves the bare name to this file's own schema. The remaining two cases assert the exact array (not toContain: the defect adds a row, so any assertion satisfied by a superset is satisfied by the defect), through the arm and through introspectSchema, the in-tree consumer.

Reverse verification, predicted before each leg

Legs 1 and 3 ran with the tree byte-identical to the committed blob; leg 2 reverted only sql-driver.ts to its parent commit, proven on disk before running (new-predicate occurrences 2 to 1, comment marker absent, git diff --stat showing 20 deletions).

legpredictedobserved
fix applied3 passed3 passed
fix revertedcases 2+3 RED with the neighbour's FK present; case 1 stays green (it issues the pre-fix predicate itself)exactly that — 1 passed, 2 failed
restored3 passed3 passed

The red diff named the defect precisely, the extra row being the neighbour schema's:

+ {
+ "columnName": "there_ref",
+ "constraintName": "os11201_fk_there",
+ "referencedColumn": "id",
+ "referencedTable": "os11201_ref_there",
+ },

No rebuild happened between the legs and the behaviour still flipped, which is the positive evidence that this pin reads source, not a stale dist/ — so no dist preflight applies to it.

Verification

All gates below were run on the tree at 5c2897943, which is this PR's head commit, with a clean working tree. The gate union was derived from the actual diff via node scripts/pm/dispatch-gates.mjs with no paths passed. Each exit status was captured before any pipe.

Path-derived: check:changeset-gate-self-tests 0 · check:driver-conformance 0 · check:objectui-changeset 0 · check:published-files 0 · check:slot-lookup 0 · check:test-source-alias 0 · check:type-source-resolution 0 · check-adr-0087-registration.mjs 0 · check-changeset-no-major.mjs 0 · check-ci-filter-parity.mjs 0 · check-empty-changeset.mjs 0 · check-plugin-teardown-shape.mjs 0 · docs-audit/check-affected-docs.mjs 0

Convention-triggered (adds a test file): check:query-options-erasure 0 · check:type-check-coverage 0 · check:type-check-debt 0 · check:engine-double-contract 0 · check:cross-package-test-inputs 0 · check:where-matcher 0. check:type-check-debt was run after turbo run build --filter='./packages/*' --filter='./packages/*/*', exactly as lint.yml sequences it, so it measured rather than refused; it reported 33 ledger entries re-measured, 1897 raw tsc errors, none above its recorded number.

Also run: check:nul-bytes 0, pnpm --filter @objectstack/driver-sql typecheck 0, and the full package suite against the live server — 120 files passed, 1 skipped; 2153 tests passed, 45 skipped, 0 failed under TZ=America/New_York with OS_TEST_POSTGRES_URL set.

Live MySQL was not exercised locally and is not claimed: the diff does not touch the MySQL branch, and CI's Temporal Conformance (live PG + MySQL) job covers that cell.

Deliberately not done

Two further defects in this same query's JOIN correlations were measured on the live server and are filed as #11324 rather than fixed here — they are a different class from this card's unscoped read, each needs its own fixture, and the likely repair is a pg_constraint rewrite that would cross into the interface region another PR owns. Both were confirmed present after this change, so this PR neither causes nor repairs them:

  • a foreign key whose target table lives in another schema returns zero rows and vanishes from the answer (ccu.table_schema = tc.table_schema demands parent and child share a schema);
  • a 2-column composite foreign key returns 4 rows, the cartesian product of child columns by parent columns (no ordinal correlation between kcu and ccu).

Region discipline held: both hunks land at ~13151 and ~13167, inside the query body. The Introspected* interface declarations (3673-3729, 12925-12934) that PR #11270 owns are untouched, IntrospectedForeignKey's shape is unchanged, and #11224's write-door stamp region is untouched.


Generated by Claude Code

…on's schemas (#11201)
`information_schema.table_constraints` spans every schema the session has
privilege on, so filtering only on `tc.table_name = ?` merged a same-named
table's foreign keys from schemas `search_path` never reaches. Add the pin the
rest of the family already carries — `AND tc.table_schema = ANY
(current_schemas(false))` — spelled and placed as `introspectUniqueConstraints`
spells it.
Regression pin against a live PostgreSQL 16.13: two same-named tables in two
schemas, each with a different foreign key. The MySQL arm already pins
`TABLE_SCHEMA = DATABASE()`; SQLite has no schemas.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json b9e9227e36d8964a60bb4e0614c1300bedd1fd51packageMentionDocs.

Which tree this was computed on

This run read content/docs from 6155748a789cd2fe6f4eb6dd227388c7a569b7cd — the merge of head 5c2897943692fed5d0494c21398cfacfe81d6f92 into base b9e9227e36d8964a60bb4e0614c1300bedd1fd51, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 6155748a789cd2fe6f4eb6dd227388c7a569b7cd && git checkout 6155748a789cd2fe6f4eb6dd227388c7a569b7cd
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b9e9227e36d8964a60bb4e0614c1300bedd1fd51 5c2897943692fed5d0494c21398cfacfe81d6f92 && git checkout -B drift-repro b9e9227e36d8964a60bb4e0614c1300bedd1fd51 && git merge --no-ff 5c2897943692fed5d0494c21398cfacfe81d6f92
node scripts/docs-audit/affected-docs.mjs --json b9e9227e36d8964a60bb4e0614c1300bedd1fd51

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

ACCEPT — engine seat. Marked ready for review, then enqueued (that order deliberately: ready_for_review clears auto-merge and any queue slot, so undrafting after enqueuing would silently discard it).

Green read by job name

All 31 check runs completed, every one success; Build Docs and Console Pin Gate skipped for cause — no docs and no .objectui-sha in this diff. Skipped is not red, which matters on this file since a red Console Pin Gate has been a real signal since #11146.

Temporal Conformance (live PG + MySQL) passed, which is the job that carries this card: the new pin is declared through declareDialectCell(PG_CELL, ...), so it runs against the real postgres:16 service container rather than resting on the author's local server.

Why this review took the test seriously and not just the predicate

The one-line fix is not the interesting part — the pin is, because the headline assertion is an absence, and an absence passes for free on a fixture that never collided. Three things in this PR close that hole, and they are the reason I am accepting without asking for more:

  • A non-vacuity case that arms the others: case 1 re-issues the pre-fix predicate verbatim and requires it to see both schemas' constraints, plus confirms to_regclass resolves the bare name to this file's own schema. If the fixture ever stops colliding, case 1 goes red and says so — the absence assertions can no longer go green by accident.
  • Exact-array assertions rather than toContain, because the defect adds a row: anything satisfied by a superset is satisfied by the defect itself.
  • A reverse-verification whose prediction was non-obvious and held: under revert, cases 2+3 red but case 1 stays green — precisely because it issues the pre-fix predicate. Observed 1 passed / 2 failed, with the extra row named in the diff (os11201_fk_there). The mutation was proven on disk before the leg (predicate count 2→1, comment marker absent, 20 deletions) and the restore proven byte-identical.

Also correct: no rebuild happened between legs and behaviour still flipped, which is positive evidence the pin resolves source rather than a stale dist/ — so no dist preflight applies here. That is the right way to retire a caveat: by measurement, not by assertion.

Scope held where it was asked to

needs:contract-review does not apply: packages/spec is not in the diff, no interface shape changed, and the accepted set is narrowed rather than widened — a same-named table in another schema stops contributing foreign keys it never should have contributed. That is declared = enforced restored.

Follow-up filed, not smuggled in

#11324 — two further JOIN-correlation defects in this same query, measured on live PG 16.13 after this fix, so this PR neither causes nor repairs them: a cross-schema FK target returns zero rows (the constraint vanishes), and a composite FK returns a cartesian product. Filing rather than fixing was right — different defect class, separate fixtures, and the likely pg_constraint + WITH ORDINALITY repair could reach the declaration region #11270 owns. I have asked triage for priority first-grading there, with the population question explicitly marked unmeasured.

H17 touch-notices for this PR are posted on #8740 and #6009; neither symbol-scoped restart condition fires.


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-zhuang@claude