fix(driver-sql): the hash-shadow arm survives a plain unique over duplicate rows - #16288
Conversation
…licate rows `syncDeclaredIndexes`'s `catch` handles a refused unique on two arms: the direct one, and the hash-shadow one MySQL takes when a key part exceeds the 768-char utf8mb4 ceiling. #14902 brought the direct arm to the ADR-0120 D4 disposition -- a uniqueness violation over existing rows is a durability degradation, not a fatal. The shadow arm still required a NULL-safe organization key part as well, so a PLAIN unique matched neither branch, fell through to the unkeyable-column refusal and took the boot down. Measured on live MySQL 8.0.46: the boot died with ER_BLOB_KEY_WITHOUT_LENGTH, advising a `maxLength` the field already declared, naming neither the two duplicate rows nor a remedy. Not a bare guard widening. The surviving branch's message says the rows violate the NULL-safe key and duplicate what the previous void constraint admitted (#5030); neither clause is true of a plain unique. The two arms are split so the NULL-safe one keeps its wording and the plain one carries the direct arm's reviewed sentence. Verified on live MySQL 8.0.46 through a new opt-in cell, with the two existing shadow cells as the firing control (green before and after). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
📓 Docs Drift CheckThis PR changes 1 package(s): 7 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 — 10 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 120dee100e6dffa686e89cdd97160dbf040b95d8 && git checkout 120dee100e6dffa686e89cdd97160dbf040b95d8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin bda7b7c83b82c09f229ffd613c6900200089189a 895d55979f255bb0c5da6469553f9095e86cb0a2 && git checkout -B drift-repro bda7b7c83b82c09f229ffd613c6900200089189a && git merge --no-ff 895d55979f255bb0c5da6469553f9095e86cb0a2
node scripts/docs-audit/affected-docs.mjs --json bda7b7c83b82c09f229ffd613c6900200089189a
|
Fixes #15479
syncDeclaredIndexeshandles a declared unique the database refuses in onecatch, and thatcatchhas two arms: the DIRECT one, and the hash-shadow one MySQL takes when a key part exceeds the 768-char utf8mb4 ceiling (#11627). #14902 / PR #15477 brought the direct arm to the ADR-0120 D4 disposition — a uniqueness violation over existing rows is a durability degradation, not a fatal. The shadow arm kept the older guard,nullSafe.size > 0 && isUniqueViolationError(shadowErr), so a PLAIN unique (tenancy: { enabled: false }, or an explicitunique: 'global') matched neither branch and fell through to the unkeyable-column refusal, which throws.The card said this was unmeasurable here. That was the one thing it got wrong, and correcting it is most of this PR
The card filed the gap rather than a fix because "the dispatch container has no MySQL", and was explicit that its reachability claim was "derived by reading the code, not measured". That is a good instinct on a durability path and it should stay cheap to file — but the constraint was not real. MySQL 8.0.46 installs from the distro archive in this container (
apt-get install mysql-server,mysqldstarted directly since there is no systemd), and every reading below is against that live server. No Docker daemon is running here; none was needed.The premise is therefore no longer derived. It is measured, and it reproduces.
Before — the boot dies, and its advice is actively wrong
On the unmodified tree, with two rows already sharing a 900-char value and a plain unique newly declared over them,
initObjectsthrows:Note what the operator is told: declare a
maxLength— on a field that already declaresmaxLength: 1024. The message is about the wrong problem entirely. The actual cause is two duplicate rows, and it names neither them nor a remedy. That is the shape #14902 graded p1 on the sibling arm.After — the three acceptance conditions from the card
Same fixture, same server, after the change — this line is quoted verbatim from a live run, not composed here:
initObjectsresolves);information_schema, never from our own DDL).Not a guard widening — the two-arm message split
The surviving branch's message said the rows "violate the NULL-safe key (duplicates the previous void constraint admitted, #5030)". Neither clause is true of a plain unique: nothing admitted these rows, and there is no NULL-safe key. Widening the guard while leaving one message would ship a factually false durability log — worse than the throw it replaces, because it sends the operator hunting a NULL-distinct index that never existed and an #5030 defect that never occurred.
So the arms are split. The NULL-safe branch keeps its wording untouched; the plain branch carries the direct arm's already-reviewed sentence verbatim, and reuses
formatDuplicateGroups— the module-local helper #14902 added precisely so the two sites reporting a blocked unique cannot drift into two spellings. The only word that is this arm's own is the route noun "hash-shadow", because that is what honestly names which physical route was attempted, and the NULL-safe branch above already spells it that way.The uniqueness limb comes from the enclosing
if (unique)a few lines above, so the new guard asks only the violation question. Verified that the enclosing condition really isuniqueat that nesting level rather than assumed.Measurements
Environment: MySQL
8.0.46-0ubuntu0.24.04.4, started directly (mysqld --bind-address=127.0.0.1), reached over TCP throughOS_TEST_MYSQL_URLand this package's per-file schema isolation (#9350). All runs at895d55979.Firing control, taken FIRST, on the unmodified tree — without it a green run afterwards proves nothing. The two existing opt-in cells named on the card: 15 passed, 1 skipped. The single skip is the un-provisioned Postgres cell, not a MySQL one — 12 live-MySQL tests genuinely executed and drove real DDL.
Red, then green. The new cell
sql-driver-15479-shadow-plain-unique-duplicates.test.tson the unmodified source: 2 failed / 2 passed — the two "boot survives" blocks failed with theER_BLOB_KEY_WITHOUT_LENGTHthrow quoted above. After the change, with both existing cells and #14902's suite alongside: 27 passed, 1 skipped (4 files), same lone Postgres skip.The new cell carries four blocks, two of which exist to catch the plausible wrong fixes: a CONTROL asserting the NULL-safe arm still says the NULL-safe thing (a single shared message fails here), and a positive control that a plain shadow unique over CLEAN data is still created and still enforces (a guard that turned the shadow route off fails here). Both shapes that reach the plain path are covered, because a guard keyed on only one of them would pass locally and fail in production.
Whole package, no live URL: 155 files passed / 10 skipped, 2389 tests passed, 141 skipped.
typecheckgreen — and proven to actually cover the change rather than excluding tests:tsc --listFilesputs both edited files in the 577-file checked set.Published surface (clause ②):
no, measured by ablation rather than judged. Built at head, swappedsql-driver.tsback to the merge basedacb73f4f, rebuilt, compared every declaration file the package'sfiles[]publishes (dist/index.d.ts,dist/index.d.mts), then restored — restore proven by blob equality against the HEAD blob and an emptygit diff HEAD. Exported names 73 → 73, class/interface members 349 → 349, nothing added, nothing removed. The bytes do differ, so the delta is classified rather than waved through: one hunk, 6 added lines, 0 removed, all six JSDoc comment lines, with the declarationprotected syncDeclaredIndexes(…): Promise<void>;byte-identical on both sides. The instrument was then proven live against a change that SHOULD move the surface — a throwawayprotected zzControlProbe15479moved members 349 → 350 and was named by the extractor — so the 349 → 349 reading is a measurement, not a blind spot.Gates: the 54 families derived by
scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackfrom the real change set (the script's own three-dot derivation, no hand-built file list). 53 ran, all green at895d55979;check:partof-closing-keywordreadsPR_BODYfrom the environment and is run against this posted body rather than an empty one. Three of them (check:published-readme-exports,check:dual-build-cjs-loads,check:type-check-debt) first returned their explicit exit 3 — "nothing was measured" — so the full 71-package closure was built and all three re-run to a real green. And because a gate's green can be about a population that excludes your package,check:dts-closurewas controlled: removingdriver-sql/dist/index.d.tsturns it red naming@objectstack/driver-sql, restoring it (byte-identical) turns it green.Out of scope
os migrate planis untouched — #15477's D4 pre-flight is guard-free and already reports this opdestructivewith the row report.Changeset:
patchon@objectstack/driver-sql. Behaviour on every other dialect is unchanged; SQLite and Postgres never refuse the direct index and so never reach this arm.This PR was produced by the
domain:engineexecution seat in sessionsession_01ARYe3yQTQCUFm5qPYNgKaJ, on branchclaude/issue-15479-mysql-hash-shadow-plain-unique.🤖 Generated with Claude Code
https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
Generated by Claude Code