Skip to content

fix(driver-sql): a merge-path upsert no longer rewrites an existing row's autonumber (#7011) - #7059

Merged
os-project-manager merged 1 commit into
mainfrom
claude/issue-7011-upsert-autonumber-immutable
Aug 9, 2026
Merged

fix(driver-sql): a merge-path upsert no longer rewrites an existing row's autonumber (#7011)#7059
os-project-manager merged 1 commit into
mainfrom
claude/issue-7011-upsert-autonumber-immutable

Conversation

@os-project-manager

Copy link
Copy Markdown
Collaborator

Fixes #7011.

The ruling this PR implements (triage, 2026-08-09, quoted per the dispatch)

an autonumber is an immutable business identifier once assigned — the fix excludes auto_number columns from mergeColumns so a merge never rewrites an existing row's number; the pre-burn half (a number consumed before insert-vs-merge is known) belongs to #6943's reseed family and is explicitly out of scope here. If implementation finds any consumer relying on merge-overwrite, STOP and report.

Premise re-verified on the branch base (main @ 3fc2e48, healthy counter, one row)

The new pin file was run before the fix, and went red with exactly the filing's values:

create                      → CASE-00001    last_value 1
upsert same id (1st time)   → CASE-00002    last_value 2   (asserted CASE-00001, received CASE-00002)
upsert same id (2nd time)   → CASE-00003    last_value 3

One row throughout; its case_number was rewritten twice. Cause, as filed: fillAutoNumberFields reserves a number before the statement knows whether it will insert or merge, and the autonumber column sat in mergeColumns, so the ON CONFLICT … DO UPDATE branch wrote the fresh reservation over the row's existing number.

The fix

SqlDriver.upsert() now derives its merge column list through a new insertOnlyUpsertColumns(object) helper: created_at (the pre-existing exclusion, byte-identical behavior) plus every auto_number column of the object, resolved under the same object-name/table-name lookup fillAutoNumberFields uses and mapped to physical column names (external objects can remap logical fields via external.columnMap, and the merge list is derived from the applyWriteColumnMap-processed row).

The exclusion is unconditional: an explicit autonumber value in the upsert payload does not renumber an existing row on the merge branch either — update() writes what it is given and remains the deliberate renumbering path. Insert-path upserts still assign fresh numbers, and every non-autonumber column (including updated_at) merges exactly as before.

Consumer check (the STOP condition) — no consumer relies on merge-overwrite

Every driver-level upsert caller in the monorepo was enumerated:

  • Lifecycle Archiver (packages/objectql/src/lifecycle/lifecycle-service.ts:1110, cold.upsert(object, row, ['id'])) — copies full hot rows carrying their autonumber values explicitly. The merge branch only re-fires when an interrupted sweep re-copies the same unchanged row (archived rows are hot-deleted after copy), so the excluded column's payload value equals the stored value and the exclusion is observably a no-op there.
  • packages/runtime/src/sandbox/body-runner.ts:276 guards on typeof ql.upsert === 'function'ObjectQLEngine exposes no upsert method, so this path routes to ql.insert today.
  • REST import's writeMode: 'upsert' is find-then-create-or-update at the engine level (import-runner.ts findExisting) and never calls driver upsert.

No fixture outside the three driver packages combines driver upsert with autonumber (swept by grep over every @objectstack/driver-sql-importing test).

Faces

face conclusion basis
SqlDriver fixed primary; new pin file
SqliteWasmDriver inherits (upsert not overridden) new merge-path pin in its existing #6943 upsert test file
TursoDriver local/replica overrides upsert, routes to super new LOCAL merge-path pin in its existing #6943 test file
TursoDriver remote neither defect nor fix RemoteTransport.upsert never enters fillAutoNumberFields (#6999's table; unchanged)

driver-memory / driver-mongodb are #5499-frozen and untouched — neither declares supports.autonumber; both use the engine fallback (#6806's surface), per the card's own boundary note.

Out of scope, measured for #6943's card (the pre-burn reading the dispatch asked for)

Post-fix, on a healthy counter: the row keeps CASE-00001 through both merge-upserts, but last_value still walks 1 → 2 → 3 — the reservation still happens before insert-vs-merge is known, so a merge-only upsert still consumes one sequence value per call. The next insert-path row gets CASE-00004, i.e. CASE-00002/CASE-00003 are now permanent gaps rather than rewrites. Deferring the reservation is #6943-family work; no test in this PR asserts last_value on the merge path, so that future fix cannot turn this PR's pins red.

Existing rows already renumbered by past merges cannot be restored from the driver side (as the filing noted).

Reverse verification (direction predicted before running)

Predicted: restoring the deleted limb (autonumber exclusion off) turns exactly the merge-path pins red, with the filing's own values; insert-path and non-autonumber-merge cases stay green. Measured: driver-sql pin file 4 failed / 1 passed (expected 'CASE-00002' to be 'CASE-00001', 'TKT-0002' vs 'TKT-0001'), sqlite-wasm file 1 failed / 2 passed (only the new #7011 pin), turso file 1 failed / 3 passed (only the new LOCAL #7011 pin). Fix restored: 5/3/4 all green.

Tests

  • pnpm --filter @objectstack/driver-sql test — 80 files passed, 4 skipped (1147 tests passed, 48 skipped)
  • pnpm --filter @objectstack/driver-sqlite-wasm --filter @objectstack/driver-turso test — 23 files / 308 tests and 32 files / 912 tests, all passed
  • tsc --noEmit (typecheck) green on all three packages; ESLint clean on the four touched files
  • Changeset: @objectstack/driver-sql patch (user-visible behavior fix)

🤖 Generated with Claude Code

https://claude.ai/code/session_01LGRN2cSRfggfX9B2L83bQc


Generated by Claude Code

…ow's autonumber (#7011)

An autonumber is an immutable business identifier once assigned (triage
ruling on the card). fillAutoNumberFields reserves a fresh number before
the statement knows whether it will insert or merge, and the autonumber
column sat in mergeColumns — so every ON CONFLICT ... DO UPDATE wrote the
freshly reserved number over the existing row's, measured on a healthy
counter: create -> CASE-00001, two upserts of the same id -> CASE-00002
then CASE-00003, one row throughout.

auto_number columns are now excluded from the merge column list, exactly
like created_at (insert-only facts about the row's birth). The exclusion
is unconditional — an explicit payload value does not renumber on merge
either; update() remains the deliberate renumbering path. Insert-path
upserts still assign fresh numbers; every non-autonumber column
(including updated_at) merges as before.

Out of scope, deliberately (#6943's reseed family): the reservation still
happens before insert-vs-merge is known, so a merge-only upsert still
consumes one sequence value per call — a permanent gap now, no longer a
rewrite (measured post-fix: last_value 1 -> 2 -> 3, next fresh row gets
CASE-00004).

Faces: SqliteWasmDriver inherits upsert; TursoDriver local routes its
override to super — both pinned by their own tests. Turso remote never
enters fillAutoNumberFields (neither defect nor fix).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LGRN2cSRfggfX9B2L83bQc
@vercel

vercel Bot commented Aug 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 9, 2026 12:41pm

Request Review

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/driver-sql.

8 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/drivers.mdx (via @objectstack/driver-sql)
  • content/docs/getting-started/glossary.mdx (via @objectstack/driver-sql)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/driver-sql)
  • content/docs/plugins/anatomy.mdx (via @objectstack/driver-sql)
  • content/docs/plugins/packages.mdx (via @objectstack/driver-sql)
  • content/docs/protocol/kernel/index.mdx (via @objectstack/driver-sql)
  • content/docs/protocol/kernel/lifecycle.mdx (via @objectstack/driver-sql)
  • content/docs/protocol/objectql/query-syntax.mdx (via @objectstack/driver-sql)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx (via @objectstack/driver-sql)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Aug 9, 2026

Copy link
Copy Markdown
Collaborator Author

PM step-7 acceptance review — PASS (session_01LGRN2cSRfggfX9B2L83bQc, maintainer-expedited v17 card).

Verified independently against head e6b0c43: (1) premise reproduced red-first with the filing's exact values before the fix; (2) ruled scope held exactly — autonumber columns excluded from mergeColumns via a helper that reuses fillAutoNumberFields' own object/table lookup and maps to physical columns, pre-burn half untouched and its post-fix reading recorded for #6943; (3) the STOP condition was discharged by enumeration, not assumption (Archiver re-merges identical rows; body-runner's upsert branch is dead; REST import never calls driver upsert); (4) reverse verification predicted-then-measured with exact hit (4/1, 1/2, 1/3 red splits); (5) CI 25/25 zero failures, independently read — including ADR maintainer approval success on a non-ADR PR, confirming the newly-required context reports everywhere; (6) five files, zero riders, changeset patch present. Marking ready and queueing.


Generated by Claude Code

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

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

upsert 每次「合并到既有行」都烧一个自增号,并覆写该行已有的业务号(健康计数器上实测,与陈旧无关)

2 participants