Skip to content

fix(driver-sql): report every member of a SQLite composite primary key, in key order - #11104

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-10997-sqlite-composite-pk
Aug 22, 2026
Merged

fix(driver-sql): report every member of a SQLite composite primary key, in key order#11104
os-zhuang merged 1 commit into
mainfrom
claude/issue-10997-sqlite-composite-pk

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#10997

SqlDriver.introspectPrimaryKeys collected SQLite primary-key columns with if (row.pk === 1). PRAGMA table_info does not report pk as a boolean — it is the column's 1-based position within the primary key (0 = not part of the key, 1 = first key column, 2 = second, and so on). The filter therefore kept only the first member of a composite key and silently dropped the rest.

Both output signals were wrong together and for the same reason: introspectSchema derives col.isPrimaryfromprimaryKeys (if (primaryKeys.includes(col.name)) col.isPrimary = true), so a consumer could not cross-check its way back to the dropped member. Repairing the list repairs the flag with it — verified in the pin rather than assumed.

The rows are now also ordered by the pk ordinal instead of being taken in table_info row order, which is column order. The two differ whenever a key is declared out of column sequence, and primaryKeys is consumed as an addressing / upsert-conflict-target key where the order is load-bearing.

Measured

Red baseline — the new pin run against the unrepaired driver:

× reports every member of a composite key, and derives isPrimary for all of them
× orders primaryKeys by pk ordinal, not by column position
× reads as NO drift when a declared composite key is compared against introspection
AssertionError: expected [ 'order_id' ] to deeply equal [ 'order_id', 'line_no' ]
AssertionError: expected [ 'shipment_id' ] to deeply equal [ 'shipment_id', 'carrier_code' ]
AssertionError: expected [ 'missing_key_member:line_no' ] to deeply equal []
Tests 3 failed | 1789 passed | 72 skipped (1864)

That second failure is independent evidence for the ordinal reading: on a table with columns (carrier_code, shipment_id, leg_seq) and primary key (shipment_id, carrier_code), the old pk === 1 filter selected shipment_id — the second column but the first key member. pk tracks key position, not column position.

After the repair, same suite: Tests 1792 passed | 72 skipped (1864).

What the pin asserts

New file packages/drivers/driver-sql/src/sql-driver-composite-primary-key-introspection.test.ts, over real in-memory SQLite (better-sqlite3):

  • The SQLite fact the repair rests on: PRAGMA table_info(order_lines) reports { order_id: 1, line_no: 2, sku: 0 }. If SQLite ever reported pk as a boolean, the repair would be wrong, and this says so out loud.
  • Both signals: primaryKeys deep-equals ['order_id', 'line_no'], and isPrimary is { order_id: true, line_no: true, sku: false }.
  • Ordering, on a table whose declared key order deliberately differs from its column order.
  • The consumer-visible consequence: a declared composite key compared against introspection reads as no drift. This one carries a negative control — the same comparison still reports missing_key_member / unexpected_key_member for a genuinely different key, and key_order_mismatch for a reordered one — so the empty result is a measurement rather than a vacuous assertion.
  • Non-regression: a single-column key still reports exactly ['id'], and an unkeyed table reports [] (that last one is what would catch the repair drifting to pk >= 0).

Dialect arms actually executed

Only the SQLite arm was executed (better-sqlite3, in-memory). The Postgres and MySQL arms were read, not run: this container has no listener on 5432/3306, no DSN in the environment, no mysql client, and the Docker CLI is present but has no daemon socket. Nothing here claims either arm was verified.

Confirming what the issue asked about them: neither orders by key position — ORDER BY is absent from both. The Postgres arm tests a.attnum = ANY(i.indkey), which reads the ordered int2vector as a set and discards the position; the MySQL arm never selects or orders by KEY_COLUMN_USAGE.ORDINAL_POSITION. Out of scope for this PR and filed separately as #11101, deliberately rather than for convenience: the whole method body sits inside catch { /* silently ignore */ } returning [], so a rewritten query that is invalid on some server version would degrade to no primary key at all with no diagnostic. Unexecuted SQL behind a silent catch is the wrong risk to take for an ordering improvement.

Inheritance

SqliteWasmDriver and TursoDriver extend SqlDriver and override neither introspectPrimaryKeys nor introspectSchema (SqliteWasmDriver additionally overrides isSqlite to true, since it passes a dialect class as client rather than a string, so the SQLite arm is genuinely selected). Both inherit the repair. Their suites pass: driver-sqlite-wasm 395, driver-turso 1006.

Output shape

Unchanged — IntrospectedTable still { name, columns, foreignKeys, primaryKeys }, primaryKeys still string[]. Only the completeness and order of the values change, so the downstream primaryKeyReader union seam in packages/services/service-datasource needs no repricing; the truncation its docblock records as "upstream of this seam and is not repaired here" is now repaired upstream. service-datasource suite passes: 559.

Verification

All at 035bcfa73.

checkresult
pnpm --filter @objectstack/driver-sql testTests 1792 passed | 72 skipped (1864), TEST_EXIT=0
pnpm --filter @objectstack/driver-sql typecheckTYPECHECK_EXIT=0
pnpm lint (repo-wide, eslint . --no-inline-config)exit 0, no findings
pnpm check:driver-conformanceOK — 45 covered cell(s), 0 in the DEBT ledger, 0 exempt.
pnpm check:slot-lookup✓ slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new
pnpm check:test-source-aliasOK — 72 packages with tests scanned
pnpm check:type-source-resolutionOK — 77 packages with a tsconfig.json scanned
node scripts/check-ci-filter-parity.mjsOK: all 83 declared cross-package glob(s) (72 unique) are covered
node scripts/check-plugin-teardown-shape.mjs✓ 63 Plugin implementation(s) across 4457 source(s)
node scripts/docs-audit/check-affected-docs.mjs✓ affected-docs self-test: 339 cases pass.
pnpm check:nul-bytes✓ self-test: 75 assertions, exit 0
pnpm check:query-options-erasure✓ ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new
pnpm check:engine-double-contractOK — 377 pinned, 133 in the DEBT ledger, 2 exempt.
pnpm check:where-matcher✓ 278 matcher(s) discovered, 278 answer the combinator battery correctly or refuse it loudly
pnpm check:type-check-coverageOK — 65/78 workspace packages type-checked
pnpm check:type-check-debt--re-measure: OK — 33 ledger entr(ies) re-measured in 388.8s, 1908 raw tsc error(s) total, none above its recorded number.
pnpm check:changeset-gate-self-testsexit 0
pnpm check:objectui-changeset✓ objectui-changeset-digest --self-test: all checks passed
node scripts/check-adr-0087-registration.mjsexit 0
node scripts/check-changeset-no-major.mjsexit 0
node scripts/check-empty-changeset.mjsexit 0

Gate families were re-derived from the actual diff with node scripts/pm/dispatch-gates.mjs (no paths — it takes its own change set from the merge base). That added five convention-triggered families beyond the dispatch lead, all of them triggered by adding a test file: check:query-options-erasure, check:type-check-coverage, check:type-check-debt, check:engine-double-contract, check:where-matcher. The type-check-debt ratchet was run against a fully built workspace closure (turbo run build, 70/70 successful), as it refuses to measure otherwise.

Changeset: .changeset/sqlite-composite-primary-key-introspection.md (@objectstack/driver-sql: patch).


Generated by Claude Code

…y, in key order
`SqlDriver.introspectPrimaryKeys` filtered `PRAGMA table_info` rows on
`row.pk === 1`. SQLite does not report `pk` as a boolean — it is the column's
1-based position WITHIN the primary key (`0` = not part of the key, `1` = first
key column, `2` = second, ...). The filter kept only the first member of a
composite key and silently dropped the rest.
Both output signals were wrong together and for the same reason:
`introspectSchema` derives `col.isPrimary` FROM `primaryKeys`, so a consumer
could not cross-check its way back to the dropped member. Repairing the list
repairs the flag with it.
The rows are now also ordered by the `pk` ordinal instead of being taken in
`table_info` row order (which is COLUMN order). The two differ whenever a key
is declared out of column sequence, and `primaryKeys` is consumed as an
addressing / upsert-conflict-target key where order is load-bearing.
`SqliteWasmDriver` and `TursoDriver` extend `SqlDriver` and override neither
method, so they inherit the repair. The Postgres and MySQL arms did not carry
this defect and are unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
@github-actionsgithub-actionsBot added size/m documentation Improvements or additions to documentation tests tooling labels Aug 22, 2026
@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 ab47f6974a275586355f06abd83a0975b8637d12packageMentionDocs.

Which tree this was computed on

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

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

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32587559799 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Build failed in 6.00s
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 81 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

os-sam pushed a commit that referenced this pull request Aug 23, 2026
…in after the driver aligned to spec
The `primaryKeyReader` docblock is the one place this seam's reasoning is
written down, and two of its statements went false when the driver was
aligned to the `packages/spec` introspection contract (`95437e7d2d`,
#10676/#10998) and when the SQLite composite-key truncation was repaired
(#10997, PR #11104).
Comment-only. No executable line changes.
- The producer table said `SqlDriver` spells the per-column signal
`isPrimary`. It emits `primaryKey` and no `isPrimary` at all.
- The `#10997` note said the SQLite composite-key truncation "is upstream of
this seam and is not repaired here". It was repaired;
`introspectPrimaryKeys` now reports every member of a composite key in
declared key order.
- The closing note said reading the extra spellings structurally was needed
"because reconciling `objectql/src/util.ts` with the spec contract is a
spec-owned change". That reconciliation has happened.
- The `refreshCatalog` seam comment repeated the stale claim that "a real
driver spells this `isPrimary` / `primaryKeys`, never `primaryKey`".
- The seam suite's second `describe` said `SqlDriver` derives `isPrimary`
FROM `primaryKeys`; it derives `primaryKey`.
The union read is UNCHANGED — all three arms stay. `table.primaryKeys` is
live and independent: after #10997 it is the only signal here that carries a
composite key in declared key order, which a per-column boolean cannot
express. The `isPrimary` arm is now recorded as a deliberate compatibility
belt rather than a bridge to a live producer, with the measurement that
decided it, so the next reader does not re-derive it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Aug 23, 2026
…in after the driver aligned to spec (objectstack-ai#11206)
The `primaryKeyReader` docblock is the one place this seam's reasoning is
written down, and two of its statements went false when the driver was
aligned to the `packages/spec` introspection contract (`95437e7d2d`,
objectstack-ai#10676/objectstack-ai#10998) and when the SQLite composite-key truncation was repaired
(objectstack-ai#10997, PR objectstack-ai#11104).
Comment-only. No executable line changes.
- The producer table said `SqlDriver` spells the per-column signal
`isPrimary`. It emits `primaryKey` and no `isPrimary` at all.
- The `objectstack-ai#10997` note said the SQLite composite-key truncation "is upstream of
this seam and is not repaired here". It was repaired;
`introspectPrimaryKeys` now reports every member of a composite key in
declared key order.
- The closing note said reading the extra spellings structurally was needed
"because reconciling `objectql/src/util.ts` with the spec contract is a
spec-owned change". That reconciliation has happened.
- The `refreshCatalog` seam comment repeated the stale claim that "a real
driver spells this `isPrimary` / `primaryKeys`, never `primaryKey`".
- The seam suite's second `describe` said `SqlDriver` derives `isPrimary`
FROM `primaryKeys`; it derives `primaryKey`.
The union read is UNCHANGED — all three arms stay. `table.primaryKeys` is
live and independent: after objectstack-ai#10997 it is the only signal here that carries a
composite key in declared key order, which a per-column boolean cannot
express. The `isPrimary` arm is now recorded as a deliberate compatibility
belt rather than a bridge to a live producer, with the measurement that
decided it, so the next reader does not re-derive it.
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
Co-authored-by: Claude <noreply@anthropic.com>
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

Development

Successfully merging this pull request may close these issues.

driver-sql: SQLite introspection reports only the FIRST column of a composite primary key (pk === 1 vs SQLite's 1,2,3… numbering)

2 participants

@os-zhuang@claude