Repository navigation
fix(driver-turso,driver-sql): a business-key upsert keeps the stored id on the remote face, and both faces answer the stored row (#21166) - #21184
Conversation
…id on the remote face, and both faces answer the stored row The remote upsert names SqlDriver.insertOnlyUpsertColumns to the transport instead of an autonumber-only lookup of its own, so the merge set leaves id and created_at alone as the local faces do. Both read-backs look the landed row up by its conflict-key values, so a merge on a business key answers the stored row instead of the payload. Claude-Session: https://claude.ai/code/session_01Ujdtvqs7ree7WyQmEDwEnG Co-authored-by: Claude <noreply@anthropic.com>
…mote-upsert-id-insert-only
📓 Docs Drift CheckThis PR changes 2 package(s): 3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 14 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 4a17d99b957b492a3ec744247b55c58d48f8f1b7 && git checkout 4a17d99b957b492a3ec744247b55c58d48f8f1b7
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 454bbb6866f64da574a29dcc1e0be320bff50189 f80af709ecacafdce2ae2a95af1d0336be1c7f62 && git checkout -B drift-repro 454bbb6866f64da574a29dcc1e0be320bff50189 && git merge --no-ff f80af709ecacafdce2ae2a95af1d0336be1c7f62
node scripts/docs-audit/affected-docs.mjs --json 454bbb6866f64da574a29dcc1e0be320bff50189
|
Contract reviewServed-tier: Read: card #21166 (body, triage 5930765705, claim 5932434922, dev report 5933523104), PR #21184 (body, the six-file list, the net diff against ① Derived judgmentsAccept set: no call is newly accepted or newly refused. Each change the diff implies, judged:
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
Fixes #21166
Clause-②: no
What this does
Remote face (
driver-turso).TursoDriver.upsert's remote branch now passesSqlDriver.insertOnlyUpsertColumns(object)toRemoteTransport.upsert. This is the same list the local faces build their merge set from, so it namesid,created_atand theauto_numbercolumns. Before, the branch passed only the autonumber columns, from a lookup of its own (remoteAutoNumberColumns, deleted here). The merge set then carried"id" = excluded."id", and an upsert keyed on a business column replaced the stored row's primary key. This is the re-key theSqlDriver.upsertdocblock forbids: "idis insert-only … the momentconflictKeysnames a business key the merged row's identity is silently replaced". There is now one list and no second copy. A remote object never renames a column, becauseremoteTableForrefuses a renaming column map before any statement. So the list's physical names are the names the transport writes.The answer (both faces). With
idinsert-only, the remote read-back by the payload'sidstopped finding the merged row and answered the payload instead. The local face already behaved that way: H3 below. Both read-backs now look the landed row up by its conflict-key values, which identify it on both legs. When a conflict key is empty in the payload, nothing can have matched, so the row was inserted and the read is byid. On the default['id']target, both readings are the same query.Hypotheses, measured
origin/mainf3b16fc2fwith the card's table on the libsql-sqlite stub (turso-local-remote-upsert-identity-parity.test.ts, run before the fix:8 failed | 5 passed):upsert({ id: 'row-NEW', email: a, title: 'edited' }, ['email'])storedid: 'row-NEW'.upsert({ email: b, title: 'edited too' }, ['email'])storedid: 'u9ZJdBf6xIsx0URs', a minted nanoid._idalias stored'row-ALIAS'.row-aandrow-b.runImport'swriteMode: 'upsert'never reachesdriver.upsert:matchFieldsthroughfindExisting→p.findData(packages/core/src/utils/import-runner.ts:574,:583).p.updateData({ id: target.id })(:941), or it creates throughp.createData(:965),createManyDataorinsertManyData(:757).ImportProtocolLike(:132) declares no upsert member, andbulk-write.tshas zeroupsertreferences.artifact.upsertKeyto that runner asmatchFields(packages/services/service-automation/src/connector-pull.ts:291,:413).driver.upsertcaller is stillLifecycleServicewith['id'](packages/objectql/src/lifecycle/lifecycle-service.ts:1405). The sandboxql.upsertfalls back toinsert, because the engine has noupsert.SqlDriver.upsertstoredos21166_seedand answeredid: 'os21166_new': the read by the payload'sidmissed, and|| toUpsertanswered the payload. That is the failing ablation leg A1 below, on both cells. MySQL was NOT MEASURED, because no server is in this container. The code path is dialect-independent.File-surface increment (declared). The claim listed
sql-driver.tsas read-only unless lifted. H3's ruling arm adds three things:packages/drivers/driver-sql/src/sql-driver.ts(nothing else in that file);sql-driver-upsert-conflict-target-dialects.test.ts;@objectstack/driver-sql: patchbeside@objectstack/driver-turso: patchin the changeset.Pins
driver-turso/src/turso-local-remote-upsert-identity-parity.test.ts, 13 cases on both faces:title, and the answer names the stored id;_idalias;created_atis insert-only, the shared list's second member;id-keyed upsert, by default and with['id']named, and a business-key upsert that inserts;driver-sql/src/sql-driver-upsert-conflict-target-dialects.test.tsadds one case to the existing pins that keep the merged row's identity. It runs on the SQLite and live-PG cells: the answer names the stored id for a supplied and for a minted payload id, and the insert-leg answer is the control.Ablations
Each fix was committed first. Each mutation went through
scripts/ablation-replace.mjs, which proved it on disk, and was restored to a HEAD-equal blob.where('id', toUpsert.id)(driver-sql rebuilt;ablation-dist-preflightsaw the marker in 2 dist files, then--absentafter the restore build)4 failed); driver-sql pin on sqlite and live postgres (2 failed, PG answered'os21166_new')created_at, parity (5 failed)created_atdropped from the remote setcreated_atalone (1 failed; stored'2001-01-01…')"id" = ?4 failed; answered'row-NEW')Tests (head
f80af709ec, merge oforigin/mainfbcc05f40; all underos-verify-lock.sh)The test runs below ran on the fix commit
7a597948b3. The merge brought onlyplugin-auditand docs files, so they were not re-run. The gate union ran onf80af709ec.driver-sqlbuild exit 0. Closurepnpm --filter '@objectstack/driver-turso^...' buildexit 0.driver-tursofullvitest run:83 passed (83)files,2231 passed | 33 skipped.driver-sqlfull suite in two halves of 109 and 108 files:104 passed | 5 skipped,1440 passed | 100 skipped;102 passed | 6 skipped,1915 passed | 88 skipped.driver-sqlfiles that call.upsert(withOS_TEST_POSTGRES_URLon a private PG 16:8 passed,125 passed | 3 skipped, live postgres RAN, live mysql NOT RUN.driver-sqlite-wasm(extends SqlDriver) upsert file:3 passed.driver-tursoanddriver-sqltypecheck exit 0.--listFilesincludes both test files.f3b16fc2f) and after: identical,50 covered cell(s), 0 in the DEBT ledger, 0 exempt.f80af709ec: 63 commands plus 4 roster gates named as ⛔ for these paths. All exit 0.check:dual-build-cjs-loadsandcheck:lean-entry-closurefirst exited 3 (PREREQUISITE NOT MET) and exited 0 after the builds they name.dispatch-gates --ran:63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN.Acceptance notes
driver-memoryalready answers the stored id on a business-key merge (memory-unique-constraint.test.ts:174expects'1'), so the SQL faces now agree with it.sqlite+pgsweep, so the MySQL cell does not run it. The MySQL read-back takes the same branch.Generated by Claude Code