Repository navigation
Description
Split out of #10997 (SQLite composite-key truncation). #10997 repaired the SQLite arm — completeness and key ordering. The PG and MySQL arms of the same method do not carry that completeness defect, but neither orders its result by key position, so the composite key they return is in unspecified row order.
packages/drivers/driver-sql/src/sql-driver.ts, SqlDriver.introspectPrimaryKeys.
What
Postgres arm:
SELECT a.attname as column_name
FROM pg_index i
JOIN pg_attribute a ON a.attrelid = i.indrelid
AND a.attnum = ANY(i.indkey)
WHERE i.indrelid = ?::regclass
AND i.indisprimaryi.indkey is an int2vector holding the key's attnums in key order. a.attnum = ANY(i.indkey) is a membership test — it reads the vector as a set and discards the position. With no ORDER BY, the row order is whatever the plan yields (in practice pg_attribute scan order, i.e. column order). So the set is right and the order is not the declared key order whenever the key is declared out of column sequence.
MySQL arm:
SELECT COLUMN_NAME as column_name
FROM information_schema.KEY_COLUMN_USAGE
WHERE TABLE_SCHEMA = DATABASE()
AND TABLE_NAME = ?
AND CONSTRAINT_NAME = 'PRIMARY'KEY_COLUMN_USAGE.ORDINAL_POSITION is exactly the key ordinal and is selected by neither the projection nor an ORDER BY. InnoDB tends to return ordinal order in practice, but nothing in the query requires it.
Why it matters
Same reason the ordering half of #10997 mattered: primaryKeys is consumed as an addressing / upsert-conflict-target key (federated-object codegen, the persisted external_catalog under ADR-0015, and schema-drift comparison against a declared key). For those consumers a key in the wrong order is a different key. After #10997 the SQLite arm reports declared key order and these two do not, so the same table introspected through different dialects can disagree.
Likely shape of the fix
- MySQL: add
ORDER BY ORDINAL_POSITION. - Postgres: join against the ordinality of
indkeyrather than testing membership — e.g.unnest(i.indkey) WITH ORDINALITY(orgenerate_subscripts) joined topg_attribute, withORDER BYon the ordinal.
Why this was NOT fixed in #10997's PR
Two reasons, both about verification rather than difficulty:
- Neither arm is executable in the agent container — no listener on 5432/3306, no DSN in the environment, no
mysqlclient, and the Docker CLI is present but has no daemon socket (/var/run/docker.sockabsent). The original filer of driver-sql: SQLite introspection reports only the FIRST column of a composite primary key (pk === 1vs SQLite's 1,2,3… numbering) #10997 hit the same limit. Landing a rewrittenpg_indexquery that has never been parsed by a real server is a guess, not a fix. - The whole method body is wrapped in
catch { /* silently ignore */ }and returns[]on error. A rewritten query that is syntactically invalid on some PG version does not fail loudly — it degrades to no primary key at all, with no diagnostic. That turns a cosmetic ordering gap into total, invisible loss of the key. Unexecuted SQL behind a silent catch is the wrong risk to take for an ordering improvement.
So this needs a seat with a reachable Postgres and MySQL. Worth pairing with a look at whether that silent catch should stay swallowing, which is arguably the more serious finding here and is deliberately not folded in.
Not measured here
Both arms were read, not run. No claim is made about what a live server actually returns for either query — only about what the SQL as written does and does not guarantee.
Activity
os-zhuang commentedon Aug 22, 2026
First-touch: promoted
finding→pm:queue, Bug,domain:engine(packages/drivers/driver-sql, same method #10997 repaired for SQLite). Rationale: after #10997 the SQLite arm reports declared key order while PG/MySQL arms return unspecified order — the same table introspected through different dialects can disagree, and the consumers (upsert conflict targets,external_catalogunder ADR-0015, drift comparison) treat a differently-ordered composite key as a different key.Binding dispatch constraints, from the card's own reasons for not folding this into #10997: ① hard serial after #10997 — same method body, #10997 is
pm:dispatchedwith an in-flight claim; do not claim this until that lands, then re-verify line anchors on the merged ref. ② The fix must not land unexecuted SQL behind the method's silentcatch { return [] }— a syntactically bad query degrades to no key at all with no diagnostic. Acceptable execution evidence, in order of preference: the repo's PG/MySQL integration rig if one exists in CI; otherwise a seat with a reachable Postgres+MySQL (agent containers have neither daemon, as the card measured). If neither is reachable, deliver the MySQL arm (ORDER BY ORDINAL_POSITION— plain information_schema, low syntax risk) only with the PG arm reported as needs-execution, rather than landing an unparsedunnest … WITH ORDINALITYrewrite. ③ The silent-catch posture itself is deliberately out of scope — file it as its own finding if not already recorded. Size S code, M verification.Generated by Claude Code
os-zhuang commentedon Aug 22, 2026
Constraint update, same round: #10997 CLOSED at 17:38Z (its PR merged) — the hard-serial condition in the grading comment above is already satisfied. The claiming seat's remaining obligation from that clause is only the second half: re-verify this card's line anchors and the SQLite arm's landed shape on the merged ref before editing.
Generated by Claude Code
os-zhuang commentedon Aug 22, 2026
Claim: PM loop round 5
Session:
session_01RfyXxZ2WPjcjhuXpiQQc3yBranch:
claude/issue-11101-introspect-pk-key-order-pg-mysqlWorktree:
objectstack-issue-11101Domain:
domain:engineFile surface:
packages/drivers/driver-sql/src/sql-driver.ts(SqlDriver.introspectPrimaryKeys, the PG and MySQL arms) + a live-dialect test + changeset (stop on breach; explain in the report)Container & model:
M,mode:subagentClause-②: no — this corrects the ORDER of an introspected key. It changes no accept/reject behaviour and widens no public surface.
Serial constraints cleared:
sql-driver.tsis free. See the correction below — this seat had been holding it wrongly. Same-file neighbour #11067 NOT dispatched (alsosql-driver.ts, genuinely serial). Open lane PRs #11147 / #11153 / #11156 / #11139 touch four other files in three other packages; none touches this one. Lane in-flight at claim: 0.⛔ This card's stated blocker is dissolved — and I filed that blocker myself, so I am correcting it
This card (filed by this seat) concluded it could not be worked:
Every word of that is still true about the container. It is not true about the repo. Measured on
origin/main:.github/workflows/ci.ymljobtemporal-conformance, nameTemporal Conformance (live PG + MySQL), runs service containerspostgres:16(5432) andmysql:8.0(3306) with health gates, and its final step ispnpm --filter @objectstack/driver-sql testwithOS_TEST_POSTGRES_URL=postgres://postgres:postgres@127.0.0.1:5432/postgresandOS_TEST_MYSQL_URL=mysql://root:root@127.0.0.1:3306/conformance.check:required-contextspins the literal, parenthetical and all, finding: lint.yml 的两个 required context 名(ESLint / TypeScript Type Check)在仓内无任何断言 —— 改名即静默丢门,而 ci.yml 的同类名字有 check:shard-attestation 守着 #6865), so it cannot be skipped past.live-dialect-matrix.testkit.tsplus a globalsetup, with 14 driver-sql test files gating on those two env vars. A live cell is reported rather than silently passing when the URL is absent.So the rewritten SQL WILL be parsed by a real PG 16 and a real MySQL 8.0 before this can merge. The guess this card refused to take is not the guess on offer any more. I was wrong to write the blocker as a property of the work rather than of my container, and the cost was a Bug sitting unqueued.
⛔ The silent catch makes the acceptance criterion unusual — read this before writing the test
The card's second reason still stands in full, and it dictates how the test must be written:
Therefore: the test must assert the positive, ordered result — the actual key columns in declared key order. ⛔ A test that asserts "does not throw", or that only checks membership, is worthless here: a query broken on a live server produces
[]under that catch and such a test stays green over total key loss. The pin must fail loudly in the direction of the degradation.The fixture must declare a composite primary key OUT OF COLUMN SEQUENCE — e.g. columns
(a, b, c)withPRIMARY KEY (c, a). That is the only shape where key order and column order differ, so a table whose key happens to follow column order proves nothing. Assert the exact array, not a set.Direction (the card's own, not new)
ORDER BY ORDINAL_POSITION(the column is inKEY_COLUMN_USAGEand is currently selected by neither the projection nor an order clause).a.attnum = ANY(i.indkey)is a membership test that discards position. Join against the ordinality ofindkey—unnest(i.indkey) WITH ORDINALITYorgenerate_subscripts— and order by that ordinal. Verify the spelling you choose actually runs on PG 16; ⛔ do not assumeWITH ORDINALITYcomposes the way you expect inside this query without seeing it green.Cross-check your result against the SQLite arm, which #10997 already fixed for both completeness and ordering (
95437e7d2… thekeyedRows.sort((a, b) => a.pk - b.pk)shape). After this card all three dialects must agree on the same table; today SQLite reports declared key order and the other two do not, which is the actual user-visible defect (primaryKeysis consumed as an addressing / upsert-conflict-target key, so a key in the wrong order is a different key).⛔ Boundaries
catch. The card names it as "arguably the more serious finding here and is deliberately not folded in" — and it is right. Changing swallow-to-throw is an error-contract change with its own blast radius and its own ruling. File it as afindingwith your evidence and leave the catch alone. If your test happens to demonstrate what the catch hides, put that in the finding — it will be the best evidence anyone has.sql-driver.ts— driver-sql:updated_atis never refreshed on a deployment that skips boot schema sync —tablesWithTimestampsis also only filled by DDL #11067 is queued against this same file and is next in line.packages/specoff limits; ⛔ never editcontent/docs/releases/; breaking changes ship asminor.Correction to this seat's serialization, which is why this is dispatchable today
I held
sql-driver.tsfor #11009 on a declared file surface that named it. PR #11147 landed 16 files andsql-driver.tsis not among them — the fix lived in the shared dispatch predicates and the driver never needed touching. So this card and #11067 were serialized against a file that was never at risk. A declared surface is a prediction; when the actual surface lands narrower the hold should be released, not left standing to the merge. Recorded on #11009 as my error, and acted on here.Generated by Claude Code
fix(driver-sql): introspectPrimaryKeys returns the PG and MySQL key i…
os-zhuang commentedon Aug 22, 2026
os-dev-report
{ "issue": 11101, "status": "done", "branch": "claude/issue-11101-introspect-pk-key-order-pg-mysql", "pr": "https://github.com/objectstack-ai/objectstack/pull/11164", "premise_still_valid": true, "summary": "Both arms fixed and MEASURED on live servers, not reasoned about. Postgres: replaced the membership test `a.attnum = ANY(i.indkey)` with a join against the ordinality of indkey (`CROSS JOIN LATERAL unnest(i.indkey) WITH ORDINALITY AS k(attnum, ord)` + `ORDER BY k.ord`). MySQL: added `ORDER BY ORDINAL_POSITION`. New test `sql-driver-primary-key-order-dialects.test.ts` runs ONE DDL on all three dialects through `declareDialectCell` and asserts the exact ordered array; all three now agree. IMPORTANT CORRECTION for future dispatches: the card's blocking premise ('needs a seat with a reachable Postgres and MySQL') was true of the CONTAINER but not of the work — postgresql-16 (16.13) was ALREADY INSTALLED here and merely stopped (`pg_ctlcluster 16 main start`), and mysql-server-8.0 (8.0.46) installs from the distro archive. Both match CI's majors, so every before/after number below is a LOCAL live measurement, not a CI prediction. The silent `catch` was NOT touched (filed as #11161, which also found that #7332 already ruled this exact question for introspectIndexes via `onFailure: 'throw' | 'partial'` and the ruling was never applied to the three sibling methods).", "tests": "ALL AT FINAL COMMIT 9e7e37509 (tree clean, `git diff HEAD` empty, verified before each run). (1) FULL SUITE, live servers, CI's exact env — `bash scripts/pm/os-verify-lock.sh -c \"TZ=America/New_York OS_TEST_POSTGRES_URL=... OS_TEST_MYSQL_URL=... OS_EXPECT_LIVE_DIALECT_MATRIX=1 pnpm --filter @objectstack/driver-sql test\"` => `Test Files 113 passed (113) / Tests 2363 passed (2363)`, `os-verify-lock: VERDICT command-exit 0`. OS_EXPECT_LIVE_DIALECT_MATRIX=1 makes a missing URL fatal, so no live cell skipped silently. (2) NEW FILE, verbose => `Tests 14 passed (14)`: 4 sqlite + 4 'live postgres' + 4 'live mysql' + 2 catalog pins, each named in the output. (3) ABLATION — fix reverted via `git checkout origin/main -- packages/drivers/driver-sql/src/sql-driver.ts`, same servers => `Tests 4 failed | 10 passed (14)`, VERDICT command-exit 1. NO REBUILD WAS NEEDED OR DONE, and that is load-bearing rather than an omission: this package's tests import `../src/index.js`, which from a file inside `src/` resolves to `src/index.ts` — vitest reads SOURCE, never `dist/`, so a stale dist cannot mask a mutation here. The ablation going RED is itself the proof the mutated source is what executed; a dist-shadowed ablation would have stayed green. MUTATION CONFIRMED ON DISK anchored on the specific text, both directions, before the run: `WITH ORDINALITY`=0, `ORDER BY ORDINAL_POSITION`=0, `attnum = ANY(i.indkey)`=1 (the restored membership test). RESTORE leg confirmed the same way AFTER: `WITH ORDINALITY`=1, `ORDER BY k.ord`=2, `ORDER BY ORDINAL_POSITION`=1, plus `git diff HEAD` empty = byte-identical to the commit. The whole ablation ran under `trap '<restore>' EXIT INT TERM` (it fired and printed 'TRAP: restored'). PREDICTED DIRECTION STATED BEFORE RUNNING AND MATCHED EXACTLY: 4 red = the two ordered legs on each of the two live dialects; sqlite stayed green (already fixed by #10997), as did the catalog-fact legs and the per-column primaryKey-flag legs. MEASURED VALUES, through the driver, live: PG 16.13 declared (shipment_id,carrier_code) -> BEFORE 'carrier_code,shipment_id' / AFTER 'shipment_id,carrier_code'; MySQL 8.0.46 same table, same before/after; three-part key declared (b,c,a) over columns (c,a,b,d) -> BEFORE 'c,a,b' / AFTER 'b,c,a' on BOTH live dialects. Both dialects returned exactly COLUMN order pre-fix. NOTE CONTRADICTING THE CARD: the card said 'InnoDB tends to return ordinal order in practice'; on MySQL 8.0.46 it did NOT — this was a live defect on shipping code. (4) `pnpm --filter @objectstack/driver-sql typecheck` => VERDICT command-exit 0, and the log echoes `> tsc --noEmit` so it was not a zero-match silent pass. (5) `pnpm lint` = `eslint . --no-inline-config` over the WHOLE REPO, NOT narrowed => VERDICT command-exit 0 (held 96s, waited 332s behind another agent's lock). No narrowing evidence is owed because nothing was narrowed. (6) 18 gates, each exit code captured AFTER redirecting to a file (never through a pipe), all exit 0: the 12 path-derived by `node scripts/pm/dispatch-gates.mjs` (run with NO paths, letting it derive the change set from the merge base — it reported 'change set derived from git — 3 path(s) vs merge base 26dea1495') plus the 5 convention-triggered for adding a test file, PLUS `check:adr-anchors` which I added BY HAND: `scripts/adr-anchors/packages__drivers__driver-sql__src__sql-driver.ts.json` exists (ADR-0120) and the derivation explicitly warns that such a roster sitting in a directory one of your paths is in 'is not evidence in EITHER direction'. Gate verdict lines, quoted from the gates themselves rather than from $?: `check-driver-conformance: OK — 45 covered cell(s), 0 in the DEBT ledger, 0 exempt.`; `check-engine-double-contract: OK — 383 pinned, 133 in the DEBT ledger, 2 exempt.`; `check-nul-bytes: OK (scanned 6463 text file(s) ... no raw ASCII control bytes).`; `check-test-source-alias OK — 72 packages with tests scanned`. NOT RUN LOCALLY: the rest of the CI farm (CI runs it exactly once regardless); CI convergence is the PM's read per the standing dispatch contract.", "open_questions": [], "out_of_scope_findings": [ "filed as #11161 (finding): introspectPrimaryKeys / introspectForeignKeys / introspectUniqueConstraints still swallow every error and return an empty collection, so a failed read is reported as ABSENCE — and #7332 already ruled this exact question for introspectIndexes (`onFailure?: 'throw' | 'partial'`, default throw), a ruling quoted in the same file and never applied to the three siblings. Carries the evidence this card produced, per the dispatch's instruction.", "filed as #11162: PG covering primary keys (`CREATE UNIQUE INDEX ... INCLUDE (...)` promoted via `ADD CONSTRAINT ... PRIMARY KEY USING INDEX`) report the INCLUDE'd payload column as a key member — the arm reads all of indkey and ignores indnkeyatts. Measured live: indkey='2 1 3', indnkeyatts=2, and BOTH the old and the new query return `payload`. A MEMBERSHIP defect, not an ordering one, so it fails condition (1) of the bounded in-place exemption; this PR neither introduced nor changed it.", "filed as #11163: MySQL introspectColumns returns a table's columns ALPHABETICALLY, not in declared column order, because it iterates knex `columnInfo()` — an object keyed by column name — with no ORDER BY underneath. Measured: SQLite and PG 16.13 both give `carrier_code, shipment_id, leg_seq`; MySQL 8.0.46 gives `carrier_code, leg_seq, shipment_id`. Surfaced as a genuine red while writing this card's test, which now holds column order as a constant pinned against its own DDL text." ] }Generated by Claude Code
os-zhuang commentedon Aug 22, 2026
ACCEPT — PR #11164, 31/31 checks green, enqueued
Verified by name, not by aggregate. The one that matters:
Temporal Conformance (live PG + MySQL)—success. That is the required job carrying the livepostgres:16andmysql:8.0service containers, so the rewrittenpg_indexquery was parsed and executed by a real server before merge.Lint & Repo Gatesgreen,Console Pin Gateskipped not red. Flipped to ready, auto-merge enabled.This is the strongest evidence package this lane has produced. Three things in particular:
1. It dissolved the card's blocking premise a layer deeper than I did. I unlocked this card by finding live PG/MySQL in CI. The dev found that
postgresql-16(16.13) was already installed in the agent container and merely stopped (pg_ctlcluster 16 main start), and thatmysql-server-8.0(8.0.46) installs from the distro archive — both matching CI's majors. So every before/after number in the PR is a local live measurement, not a CI prediction. Servers torn down and the worktree removed without--forceafterwards.2. The measurement contradicted the card — and the card was mine. #11101 said "InnoDB tends to return ordinal order in practice, but nothing in the query requires it." On MySQL 8.0.46 it did not: an out-of-sequence key came back in column order. Both live dialects returned exactly column order pre-fix. This was a live defect on shipping code, not a theoretical one — which is a materially different card from the one I filed.
3. The ablation proves what executed, not just that something changed. Fix reverted with
git checkout origin/main -- sql-driver.ts, mutation confirmed on disk by anchored grep counts in both directions, run under anEXIT INT TERMtrap that fired and printed its restore,git diff HEADempty afterwards. Direction predicted before running — 4 red = two ordered legs × two live dialects, with SQLite green because #10997 already fixed it — and matched exactly.⭐ The reasoning about why no rebuild was needed is worth keeping as a reusable argument: this package's tests import
../src/index.js, which from insidesrc/resolves tosrc/index.ts, so vitest reads source and a staledist/cannot mask a mutation. The ablation going red is itself the proof that the mutated source is what ran — a dist-shadowed ablation would have stayed green. That converts a potential gap into a positive control.The fixture has teeth: a genuine permutation (
primary key (b, c, a)over columns(c, a, b, d)), so an implementation that merely reversed or sorted the rows fails too — plus a non-vacuity guard so that a later edit flattening the key back into column sequence fails loudly instead of quietly turning every assertion into a tautology the buggy query also satisfied. And the row order of the unordered query is deliberately left unpinned, because it is unspecified by both engines and pinning the reversal these particular servers produced would pin behaviour neither vendor promises. That restraint is correct.The silent
catchwas left alone exactly as bounded, and every assertion is positive and ordered so a degradation to[]cannot pass.Out-of-scope findings — all three verified present and labelled
findingintrospectIndexesswallows every error and returns a partial index list, which the drift differ then reports as missing indexes #7332 already ruled this exact question forintrospectIndexes(onFailure?: 'throw' | 'partial', defaultthrow) and the ruling was never applied to the siblings. That makes it declared ≠ enforced / already-ruled-but-unapplied — restore-invariant — rather than an open error-contract decision. Routing note left on the card so the grading round does not have to rediscover it.INCLUDEd payload column as a key member (indnkeyattsignored). A membership defect, measured identical before and after, so this PR neither introduced nor changed it.introspectColumnsreturns columns alphabetically; surfaced as a genuine red while writing this card's test.Two facts recorded to the seat post
sql-driver.tshas an ADR-0120 anchor roster thatdispatch-gates.mjspath derivation does not name, socheck:adr-anchorsneeds adding by hand when touching that file — the dev caught that itself. And the container/CI live-database capability, which retires "unverifiable here" as a reason to shelve a driver-sql card.Generated by Claude Code
fix(driver-sql): bound the PG primary-key join to indnkeyatts so INCL…
fix(driver-sql): introspectPrimaryKeys returns the PG and MySQL key i…
fix(driver-sql): introspection trio — error contract (objectstack-ai#…