Skip to content

fix(driver-sql): correlate PG introspectForeignKeys on the constraint and the key ordinal - #11396

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-11324-pg-fk-join-correlations
Aug 23, 2026
Merged

fix(driver-sql): correlate PG introspectForeignKeys on the constraint and the key ordinal#11396
os-zhuang merged 2 commits into
mainfrom
claude/issue-11324-pg-fk-join-correlations

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#11324

Rewrites the Postgres arm of SqlDriver.introspectForeignKeys onto pg_constraint. The three-view information_schema join it replaces was wrong in two independent ways, both reproduced here on live PostgreSQL 16.13 before a line was changed.

The two defects, reproduced not re-derived

Fixture, search_path = os11324_probe, parent in a sibling schema:

createtableos11324_far.remote_parent (id varchar(64) primary key);
createtablecross_child (
id varchar(64) primary key, p varchar(64),
constraint fk_cross foreign key (p) referencesos11324_far.remote_parent(id));
createtablecomp_parent (a varchar(64), b varchar(64), primary key (a, b));
createtablecomp_child (
id varchar(64) primary key, x varchar(64), y varchar(64),
constraint fk_comp foreign key (x, y) references comp_parent(a, b));

Issuing the arm's query verbatim:

proberesult
A. pre-fix query, cross_child0 rows — defect 1
B. pre-fix query, comp_child4 rowsx->a, x->b, y->a, y->b — defect 2

Defect 1 is ccu.table_schema = tc.table_schema. For a FOREIGN KEY, constraint_column_usage describes the referenced side — that is exactly why the projection aliases it referenced_table — so its table_schema is the parent's, not the constraint's. A cross-schema target contributes zero rows and the table reports having no foreign keys at all. [] does not read downstream as "I could not see it"; it reads as this table has no foreign keys, which federated-object codegen, the persisted external_catalog (ADR-0015) and schema-drift comparison all act on.

Defect 2 is the absent ordinal correlation between kcu and ccu. Because IntrospectedForeignKey is a flat per-column record, the two phantom pairs are indistinguishable from the real ones to every consumer.

Why the whole query moves, rather than the join predicate being patched

The card asked this to be verified rather than assumed, so it was, on the same server:

proberesult
C. ccu correlated on tc.constraint_schema, cross_child1 row — defect 1 repaired
D. same variant, comp_child4 rows — defect 2 untouched
E. columns of information_schema.constraint_column_usage7: the catalog/schema/name triples for the table and the constraint, plus column_name

So the conservative half-fix is real but partial, and probe E is why: constraint_column_usage exposes no ordinal column at all, so defect 2 has nothing to correlate on inside information_schema. That settles the split question — keeping information_schema for defect 1 would mean two different sources for two halves of one row, with the composite half still unfixable.

pg_constraint carries both facts on one row. conkey and confkey are parallel arrays in key order — measured smallint[], not int2vector, which is what makes multi-argument unnest legal here:

CROSS JOIN LATERAL unnest(con.conkey, con.confkey)
WITH ORDINALITY AS k(attnum, fattnum, ord)

Unnesting them together pairs child column with parent column by construction, and WITH ORDINALITY keeps the key position the old join threw away.

The two siblings the card pointed at

IntrospectedForeignKey's shape is NOT changed

The declaration region is free (#11270 landed), so this was a design choice rather than a scheduling one, and the card's position was adopted: a flat per-column record expresses a composite key correctly as ordered sibling rows. What was missing was not a field but a guarantee — nothing pinned the order. ORDER BY con.conname, con.oid, k.ord pins it, and the type's docblock now states the contract every arm owes: contiguous, in declared key order, each record pairing its own child column with its own parent column.

An ordinal field was considered and rejected. It would let a wrong ORDER BY keep shipping wrong rows that merely describe their wrongness, where the pairing is a fact the query itself has to get right; and it would widen an interface to record something the array index already carries. con.oid is in the ORDER BY only as a tiebreaker, so two same-named constraints in two schemas both on search_path still cannot interleave their columns.

Behaviour deliberately left alone: an unknown table name still yields an empty list rather than a throw (measured), so this does not quietly adopt the ?::regclass failure mode introspectPrimaryKeys has, and the #7332onFailure contract is untouched. Scoping is unchanged in meaning — ns.nspname = ANY (current_schemas(false)) is #11201's predicate expressed over the catalog.

Tests, and the controls that keep them from passing for the wrong reason

New: packages/drivers/driver-sql/src/sql-driver-11324-introspect-fk-join-correlations.test.ts, PG-only through declareDialectCell (neither defect is expressible on the other arms — SQLite's PRAGMA foreign_key_list and MySQL's KEY_COLUMN_USAGE both carry both sides on one row).

Both assertions are shape assertions that could pass for the wrong reason, so each is preceded by a control that requires the fixture to still exhibit the defect:

  • Cross-schema: the two tables really are in different schemas (read from pg_namespace); the constraint really exists (read from pg_constraint); and the pre-fix query, re-issued verbatim, still returns zero rows.
  • Composite:array_length(conkey, 1) is really 2; and the pre-fix query, re-issued verbatim, still returns the four cartesian pairs.
  • Ordering: a third fixture declares the key out of column sequence — foreign key (second_col, first_col) — and a control asserts key order and column order really do disagree (first_col@col2/key2, second_col@col3/key1). Without it, "the rows are in key order" would be satisfied by any query returning column order.

Each control's failure message says that the assertion beside it is measuring nothing.

Reverse verification, direction predicted before the leg

Predicted: reverting sql-driver.ts to origin/main with the tests kept turns the pair red with exactly 4 of 9 failing — the two cross-schema assertions and the two composite ones — while the 2 controls and #11201's 3 tests stay green, because the controls assert the defect is present.

Observed: Tests 4 failed | 5 passed (9), exactly those four:

AssertionError: ... expected [] to deeply equal [ { columnName: 'p', …(3) } ]
AssertionError: ... expected [ …(4) ] to deeply equal [ …(2) ]

The mutation was proven on disk before the leg (FROM pg_constraint con 1 → 0, AND ccu.table_schema = tc.table_schema 0 → 1, blob hash changed, git status --porcelain a lone unstaged M rather than MM), and the restore proven byte-identical afterwards by git hash-object matching the committed blob — not by an insertion count. No rebuild is involved on either leg: the suite reaches the subject through a relative import inside the same package, so it reads source, which the mutation changing the result demonstrates directly.

Gates

Union derived from the actual diff with node scripts/pm/dispatch-gates.mjs, no paths passed. Every family run at 574b8366d, which is this branch's final commit; each exit status captured before any pipe.

gateexit
check:changeset-gate-self-tests0
check:driver-conformance0
check:objectui-changeset0
check:published-files0
check:slot-lookup0
check:test-source-alias0
check:type-source-resolution0
scripts/check-adr-0087-registration.mjs0
scripts/check-changeset-no-major.mjs0
scripts/check-ci-filter-parity.mjs0
scripts/check-empty-changeset.mjs0
scripts/check-plugin-teardown-shape.mjs0
scripts/docs-audit/check-affected-docs.mjs0
check:query-options-erasure0
check:type-check-coverage0
check:type-check-debt0
check:engine-double-contract0
check:cross-package-test-inputs0
check:where-matcher0
check:nul-bytes0
@objectstack/driver-sqltypecheck0

Their own verdict lines rather than a bare status, for the three that speak to this diff:

check-driver-conformance: OK — 45 covered cell(s), 0 in the DEBT ledger, 0 exempt.
check-engine-double-contract: OK — 386 pinned, 133 in the DEBT ledger, 2 exempt.
check-nul-bytes: OK (scanned 6383 text file(s) ... no raw ASCII control bytes).

Repo-wide pnpm lint was not run locally; CI owns that run.

The full package suite: 2496 passed, 3 failed, and the 3 are environmental

Test Files 2 failed | 121 passed (123) / Tests 3 failed | 2496 passed (2499). All three failures are the testkit's three-way timezone-skew non-vacuity guard in sql-driver-temporal-conformance.test.ts and sql-driver-datetime-mysql-storage.test.ts — this container's Postgres and MySQL were not running when this task started and had to be started here, so they run at UTC, where CI provisions timezone=Asia/Shanghai / default_time_zone='+08:00' with TZ=America/New_York. Attempting to reconfigure the servers to CI parity was blocked in this environment.

Rather than assert that from the message alone, it was controlled: the same two files were re-run with sql-driver.ts restored to origin/main, and produced the identical 3 failures (Tests 3 failed | 153 passed (156)). Predicted before the run, matched. The tree was restored byte-identically afterwards. Nothing in this diff reaches those assertions, which read only server and process timezones. CI's Temporal Conformance (live PG + MySQL) job runs these cells against properly-provisioned containers.

Out of scope, filed rather than fixed

Neither is addressed by this PR; both remain open.

Deliberately not done


Generated by Claude Code

… and the key ordinal
Rewrites the Postgres arm of `SqlDriver.introspectForeignKeys` onto
`pg_constraint`, replacing a three-view `information_schema` join that was
wrong in two independent ways (both measured on live PostgreSQL 16.13):
1. `ccu.table_schema = tc.table_schema` demanded parent and child share a
schema. `constraint_column_usage` describes the REFERENCED side of a
foreign key, so its `table_schema` is the parent's — a cross-schema
target contributed zero rows and the table reported having no foreign
keys at all.
2. The kcu/ccu join carried no ordinal correlation, so an N-column key came
back as the N x N cartesian product (a 2-column key measured as 4 rows).
`constraint_column_usage` exposes no ordinal column, so defect 2 has nothing
to correlate on inside `information_schema`; correlating on
`tc.constraint_schema` (the spelling `introspectUniqueConstraints` carries)
was measured to fix defect 1 and leave defect 2 at 4 rows. `pg_constraint`
carries both facts on one row: `conkey`/`confkey` are parallel `smallint[]`s
in key order, so `unnest(...) WITH ORDINALITY` — the shape
`introspectPrimaryKeys` already uses for `indkey` — pairs child column with
parent column by construction and pins the key order.
`IntrospectedForeignKey`'s shape is unchanged; its doc comment now states
the ordered-sibling-rows contract the ORDER BY establishes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
…rrelation fix
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 8519095308a35d1e151944bc20cc7b9ce2c4eaf9packageMentionDocs.

Which tree this was computed on

This run read content/docs from 3d7556f5c635aa1dcde31abe03f3461b41fbec60 — the merge of head 574b8366d937e4a1de8701e5a01072e4f5f7b0c8 into base 8519095308a35d1e151944bc20cc7b9ce2c4eaf9, 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 3d7556f5c635aa1dcde31abe03f3461b41fbec60 && git checkout 3d7556f5c635aa1dcde31abe03f3461b41fbec60
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8519095308a35d1e151944bc20cc7b9ce2c4eaf9 574b8366d937e4a1de8701e5a01072e4f5f7b0c8 && git checkout -B drift-repro 8519095308a35d1e151944bc20cc7b9ce2c4eaf9 && git merge --no-ff 574b8366d937e4a1de8701e5a01072e4f5f7b0c8
node scripts/docs-audit/affected-docs.mjs --json 8519095308a35d1e151944bc20cc7b9ce2c4eaf9

⚠️ 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 discards auto-merge and any queue slot).

Green read by job name

31 check runs, 0 still running, none non-green; two skipped for cause (Build Docs, Console Pin Gate — no docs, no .objectui-sha). ⚠️ 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) — success. That one carries this card, and it also settles a question I raised on review. The PR explains its 3 local suite failures as the container's databases running at UTC where CI provisions Asia/Shanghai / +08:00. I measured PG at Asia/Shanghai while reviewing, so that explanation looked wrong — but the servers had in fact been down when this task started (the report shows pg_lsclusters16 main 5432 down, Removed stale pid file), were started here at UTC, and the Asia/Shanghai I saw came from the parallel #11321 worktree configuring them afterwards. Either way the verdict never rested on the narrative: the control did — the same two files re-run with sql-driver.ts restored to origin/main produced the identical 3 failures, predicted before the run. CI on properly-provisioned containers now confirms it independently.

For the record, that stale-server situation is my error, not the dev's: my dispatch order asserted "Live Postgres 16 is ALREADY RUNNING in this shared container", carrying forward an 11:00 reading as a 14:40 fact. Corrected in the seat post — a dispatch order must say check, and start it if needed, with CI's timezone parameters.

What earns the accept

The card asked for one thing to be verified rather than assumed — whether the whole query needed to move — and it was, on the server:

  • Probe E counted information_schema.constraint_column_usage's columns: 7, with no ordinal among them. That is what settles the split question. Keeping information_schema for defect 1 would leave defect 2 unfixable there and put two sources behind two halves of one row.
  • Probes C/D measured the conservative half-fix actually working for the cross-schema case and leaving the composite case at 4 rows — so the deliberate divergence from introspectUniqueConstraints' spelling is a measured choice, not a preference. The reason given is exactly right: for a UNIQUE constraint ccu describes the same table, so one row per column is all there is; for a FOREIGN KEY it describes a different one, and the ordinal is precisely what is missing. Matching the sibling would have fixed half this card.

IntrospectedForeignKey's shape is unchanged, and the argument for that is better than the scheduling reason it replaced: what was missing was not a field but a guarantee — nothing pinned row order. ORDER BY con.conname, con.oid, k.ord supplies it and the docblock states the contract. Rejecting an ordinal field because it "would let a wrong ORDER BY keep shipping wrong rows that merely describe their wrongness" is the correct instinct: the pairing is a fact the query must get right, not one the consumer should have to re-derive.

Every assertion here is a shape assertion that could pass on a fixture that never collided, and each is armed by a control that re-issues the pre-fix query verbatim and requires the defect to still be present. The ordering fixture declares its key out of column sequence with a control asserting key order and column order genuinely disagree — without which "the rows are in key order" would be satisfied by any query returning column order.

The reverse-verification prediction was specific enough to be falsifiable — exactly 4 of 9 red, the controls and #11201's 3 tests staying green because the controls assert the defect is present — and was observed exactly. Restore proven byte-identical by git hash-object against the committed blob rather than by an insertion count.

Two process disclosures worth naming because they were volunteered: the gate sweep was SIGTERM'd at a 10-minute foreground cap after 14 of 20, and the remainder was re-run in batches — no gate skipped; and the first on-hold symbol count returned a false zero because grep -v '^\+\+\+' is BRE where \+ is a repetition operator, caught only because the synthetic control read 0 instead of 1. That trap has now hit two devs independently today and is recorded in the seat post.

On #11377, which this PR makes reachable

Merging this converts a silent omission into a possibly-silently-wrong reference: a cross-schema FK is now returned with a bare referencedTable, and objectql/src/util.ts:209 turns that into a lookup reference that may dangle or resolve to a same-named table in the current schema.

Not holding the PR, and the reason is that no working behaviour regresses — those constraints were dropped entirely before, so nobody can have a functioning cross-schema deployment today. Both states are defects; this one is visible and #11377 records exactly how to finish it. The composite-key half is a pure win and unrelated. ⚠️ That is a judgement about direction, not a claim the new state is harmless, and I have flagged the reachability flip on #11377 for grading — including that its option 3 (refuse loudly) is the one option that would require reworking half of this PR.

needs:contract-review does not apply: packages/spec is not in this diff, no interface shape changed, and the accepted set is not widened.


Generated by Claude Code

@os-zhuang
os-zhuang added this pull request to the merge queueAug 23, 2026
Merged via the queue into main with commit 80f1dcdAug 23, 2026
32 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-11324-pg-fk-join-correlations branch August 23, 2026 16:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-zhuang@claude