Uh oh!
There was an error while loading. Please reload this page.
fix(driver-sql): introspectPrimaryKeys returns the Postgres and MySQL key in declared key order - #11164
Conversation
…n declared key order (#11101) `SqlDriver.introspectPrimaryKeys` ordered its result on exactly one of its three dialect arms. #10997 repaired SQLite (completeness and key ordering, by sorting on the `PRAGMA table_info` ordinal); the Postgres and MySQL arms returned the composite key in unspecified row order. - Postgres: `a.attnum = ANY(i.indkey)` is a MEMBERSHIP test. `i.indkey` is an `int2vector` holding the key's attnums in key order, but `ANY()` reads it as a set and discards the position, and the query carried no `ORDER BY`. It now joins the ordinality of `indkey` (`unnest(i.indkey) WITH ORDINALITY`) and orders by that ordinal. - MySQL: `KEY_COLUMN_USAGE.ORDINAL_POSITION` IS the key ordinal and was selected by neither the projection nor an order clause. It now carries `ORDER BY ORDINAL_POSITION`. Both arms were measured returning COLUMN order — the key reversed — on live servers before the fix: PostgreSQL 16.13 and MySQL 8.0.46, over a table declared `(carrier_code, shipment_id, leg_seq)` with `PRIMARY KEY (shipment_id, carrier_code)`. Notably InnoDB did NOT return ordinal order, contradicting the usual folklore. `primaryKeys` is consumed as an addressing / upsert-conflict-target key (federated-object codegen, the persisted `external_catalog` under ADR-0015, schema-drift comparison), so a key in the wrong order is a DIFFERENT key. All three dialects now agree on the same table. The method's silent `catch { return [] }` is deliberately NOT changed here — it is a separate error-contract decision with its own blast radius, filed as its own finding. It does dictate the test shape: every assertion is positive and ordered, because a query a server rejects degrades to "no primary key at all" and a does-not-throw or set-equality test would stay green over total key loss. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
📓 Docs Drift Check1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅ What this run could not see
Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 2730a9b41d8cf3b913e7e985bbc9e4125da7e8ba && git checkout 2730a9b41d8cf3b913e7e985bbc9e4125da7e8ba
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c74aefe636bfa9ba68ed4a85b319db20d9cf2907 9e7e37509c733651e5525a2da12b40c56a2118d3 && git checkout -B drift-repro c74aefe636bfa9ba68ed4a85b319db20d9cf2907 && git merge --no-ff 9e7e37509c733651e5525a2da12b40c56a2118d3
node scripts/docs-audit/affected-docs.mjs --json c74aefe636bfa9ba68ed4a85b319db20d9cf2907 |
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 32604672633 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#11101
SqlDriver.introspectPrimaryKeysordered its result on exactly one of its three dialect arms. #10997 repaired SQLite (completeness and key ordering, by sorting on thePRAGMA table_infoordinal); the Postgres and MySQL arms returned the composite key in unspecified row order.primaryKeysis consumed as an addressing / upsert-conflict-target key — federated-object codegen, the persistedexternal_catalogunder ADR-0015, schema-drift comparison against a declared key. For those consumers a key in the wrong order is a different key, and the same table introspected through different dialects disagreed. All three dialects now agree.⭐ The card's founding condition — this SQL was executed, not reasoned about
#11101 concluded it needed "a seat with a reachable Postgres and MySQL", because neither arm was executable in an agent container. That was true of the container but not of the work. Two live servers were stood up in this session and every measurement below is from them:
postgres:16postgresql-16was already installed in the container, simply not running —pg_ctlcluster 16 main startmysql:8.0apt-get install mysql-server-8.0from the distro archiveBoth were configured to CI's settings (
timezone=Asia/Shanghai/default_time_zone='+08:00'), and the suite was run atTZ=America/New_YorkwithOS_EXPECT_LIVE_DIALECT_MATRIX=1— i.e. the exact env of theTemporal Conformance (live PG + MySQL)job, with a missing URL made fatal so no live cell could skip silently.So the risk the card refused to take — landing a rewritten
pg_indexquery that no real server had ever parsed — is not being taken. The prior "unverifiable in this container" conclusion should not be carried forward to the next driver-sql card.The two rewrites
Postgres —
a.attnum = ANY(i.indkey)is a membership test.i.indkeyis anint2vectorholding the key's attnums in key order, butANY()reads the vector as a set and discards the position; there was noORDER BY. It now joins the ordinality ofindkey:Four spellings were tried against the live PG 16.13 before choosing —
unnest … WITH ORDINALITY(with and without the explicitLATERAL),generate_subscripts, andarray_position(i.indkey::smallint[], a.attnum). All four returned the declared order; the ordinality form is the one the card named. It was then re-checked against a single-column key, a keyless table, and a three-part key.MySQL —
KEY_COLUMN_USAGE.ORDINAL_POSITIONis the key ordinal and was selected by neither the projection nor an order clause. It now carriesORDER BY ORDINAL_POSITION.The fixture: a key declared OUT OF COLUMN SEQUENCE
A key that follows its columns proves nothing — column order is exactly what the unordered queries already returned. One DDL runs on all three dialects (
varchar(64)rather thantext, so MySQL will take the columns into a primary key):A second table declares
primary key (b, c, a)over columns(c, a, b, d)— a genuine permutation, so an arm that merely reversed or sorted the rows cannot pass either.What each dialect returned, before and after
Measured through the driver on the live servers. The "before" column is a real ablation run — the fix was reverted with
git checkout origin/main -- sql-driver.ts(the revert confirmed on disk by grepping for both the removed and the injected text) and the suite re-run against the same servers:(shipment_id, carrier_code)shipment_id, carrier_code✅shipment_id, carrier_code✅(shipment_id, carrier_code)carrier_code, shipment_id❌shipment_id, carrier_code✅(shipment_id, carrier_code)carrier_code, shipment_id❌shipment_id, carrier_code✅(b, c, a)c, a, b❌b, c, a✅(b, c, a)c, a, b❌b, c, a✅Both live dialects returned exactly column order — the key reversed for the two-part fixture. SQLite was already correct (#10997), which is what makes the ablation a control rather than just a red.
The ablation direction was predicted before running and matched exactly: 4 red (two ordered legs × two live dialects), with SQLite, the catalog-fact legs, and the per-column
primaryKeyflag legs all staying green.⛔ The silent catch dictates the test shape (and is NOT changed here)
The whole method body is wrapped in
catch { }and returns[]. A query that is invalid on a live server does not fail loudly — it degrades to "no primary key at all", with no diagnostic. So every assertion in the new test is positive and ordered, on the exact array. A test asserting "does not throw", or checking membership/set equality, would be worthless: it stays green over total key loss. The helper checks length first so a degradation reads as the silent catch ate the query rather than as an uninterpretable diff.Per this card's boundaries the catch itself is untouched — it is an error-contract change with its own blast radius. It is filed separately as #11161 with the evidence this work produced, including the finding that the repo already ruled on this exact question for
introspectIndexes(#7332 gave itonFailure?: 'throw' | 'partial', defaulting tothrow) and never applied the ruling to the three sibling methods. This PR does not pin the catch's behaviour in either direction.Tests
packages/drivers/driver-sql/src/sql-driver-primary-key-order-dialects.test.ts— new. Declared throughdeclareDialectCell, so the live cells are a named skip without the URLs and a hard failure underOS_EXPECT_LIVE_DIALECT_MATRIX=1; they execute for real inTemporal Conformance (live PG + MySQL), a required check. Agreement across dialects is by construction: every cell runs the same DDL and asserts against the same constant.It also carries a non-vacuity guard — if a later edit ever flattens the fixture's key back into column sequence, the guard fails rather than quietly turning every assertion into a tautology the buggy query also passed — and per-dialect catalog pins for the facts each rewrite rests on (
pg_index.indkeyis in key order;KEY_COLUMN_USAGE.ORDINAL_POSITIONis the key ordinal). The row order of the unordered query is deliberately not pinned: it is unspecified by both engines, and asserting the reversal these servers happen to produce would pin behaviour neither vendor promises.Verification — all at
9e7e37509(the final commit)pnpm --filter @objectstack/driver-sql testagainst live PG 16.13 + MySQL 8.0.46,TZ=America/New_York,OS_EXPECT_LIVE_DIALECT_MATRIX=1pnpm --filter @objectstack/driver-sql typechecktsc --noEmit, exit 0pnpm lint(eslint . --no-inline-config, whole repo, not narrowed)node scripts/pm/dispatch-gates.mjsGates run:
check:changeset-gate-self-tests,check:driver-conformance(OK — 45 covered cell(s), 0 in the DEBT ledger, 0 exempt),check:objectui-changeset,check:slot-lookup,check:test-source-alias(OK — 72 packages with tests scanned),check:type-source-resolution,check-adr-0087-registration,check-changeset-no-major,check-ci-filter-parity,check-empty-changeset,check-plugin-teardown-shape,check-affected-docs,check:query-options-erasure,check:type-check-coverage,check:engine-double-contract(OK — 383 pinned),check:where-matcher,check:nul-bytes(OK, scanned 6463 text files), andcheck:adr-anchors— the last added by hand becausesql-driver.tshas an anchor file (ADR-0120) that no path derivation names.Out of scope, filed separately
Three findings, none of them touched here:
catchacrossintrospectPrimaryKeys/ForeignKeys/UniqueConstraints, against the driver-sql:introspectIndexesswallows every error and returns a partial index list, which the drift differ then reports as missing indexes #7332 precedent.finding.INCLUDE) report the payload column as a key member;indnkeyattsis ignored. A membership defect, not an ordering one — measured identical before and after this PR, so nothing here changed it.introspectColumnsreturns a table's columns alphabetically, not in declared column order (knexcolumnInfo()is an object keyed by name). Surfaced as a real red while writing this test.Also untouched, per the card: everything else in
sql-driver.ts(#11067 is queued against the same file),packages/spec, andcontent/docs/releases/.Generated by Claude Code