Skip to content

fix(driver-sql): hash-shadow UNIQUE indexes carry the NULL-safe organization key part (ADR-0120 D3) - #13016

Merged
os-elon merged 5 commits into
mainfrom
claude/issue-12998-shadow-null-safe-key
Aug 28, 2026
Merged

fix(driver-sql): hash-shadow UNIQUE indexes carry the NULL-safe organization key part (ADR-0120 D3)#13016
os-elon merged 5 commits into
mainfrom
claude/issue-12998-shadow-null-safe-key

Conversation

@os-elon

Copy link
Copy Markdown
Collaborator

Fixes#12998

What

When MySQL refuses a declared UNIQUE index directly, the #11627 hash-shadow route now hashes the DECLARED key instead of the raw column list: syncDeclaredIndexes passes norm.nullSafeColumns through to createHashShadowUniqueIndex, and the generation expression embeds each NULL-safe organization key part (ADR-0120 D3) as COALESCE(organization_id, '__global__') inside the CONCAT. NULL-organization rows therefore fold into the global bucket and collide with each other — the same key the direct index would have enforced. Plain key parts keep the deliberate CONCAT-NULL semantics: an any-NULL tuple still conflicts with nothing.

Route taken: the card's first direction (hash the declared key). The refusal alternative would have left the two live members constraint-less on MySQL; the expression form enforces what ADR-0120 D3 already declares, and the D4 duplicate pre-flight (probeNullSafeUniqueDuplicates) already probes this exact COALESCE key, so plan-time probing and enforcement now agree (verified at source and exercised live through the new boot-diagnosis pin).

The data-dependent edge, disclosed (also in the changeset)

Turning this constraint on is data-dependent: a MySQL database that accumulated duplicate NULL-organization rows while the shadow enforced nothing fails the shadow ALTER with ER_DUP_ENTRY on its next boot. That failure is now diagnosed, never fatal and never silent: the boot continues, the durability log names the conflicting groups (probed over the same COALESCE key) and the operator action, and the constraint is honestly reported as NOT enforced until the data is deduplicated — the same disposition as the direct NULL-safe route (ADR-0120 D4). Live-measured message shape:

[sql-driver] cannot create hash-shadow unique index 'uniq_os12998_dirty_org_v' on "os12998_dirty" — existing rows violate the NULL-safe key (duplicates the previous void constraint admitted, #5030). Conflicting group(s): (organization_id="__global__", v="ddd…payload abridged…") × 2 rows. The constraint 'COALESCE(organization_id, '__global__'), v' is NOT enforced until the data is deduplicated: run "os migrate plan" for the conflicting rows (ADR-0120 D4).

The write-path duplicate diagnosis follows the key too: hashShadowSourceColumns reads the COALESCE wrapping back from the stored generation expression, and explainHashShadowDuplicate re-selects through the same fold (COALESCE on NULL-safe parts, MySQL's null-safe equality on plain parts). Without that, a genuine NULL-organization duplicate re-selected with a bare equals-NULL matched nothing and would have been misreported as a HASH COLLISION with "please report it, the write is legitimate".

Both collision directions pinned (live MySQL 8.0.46)

New file sql-driver-12998-shadow-null-safe-key.test.ts, all read back from information_schema, never from our own DDL:

  • ORG-SCOPED: two NULL-organization rows with the same payload now collide (second insert refused); a different organization or a different payload still inserts; NULL and the 'global' literal are one bucket (equivalence with the direct index's key).
  • PLAIN (the control): a plain composite's generation expression gains NO coalesce, and two any-NULL tuples still coexist. The pre-existing driver-sql (MySQL): full-value UNIQUE on >768-char token columns is inexpressible on utf8mb4 — hash-shadow-key route for the four ruled cases (C half of the #11374 ruling) #11627 pin "hashes a composite tuple, keeps any-NULL tuples non-conflicting" is untouched and stays green — deliberate semantics per MySQL's own composite-UNIQUE behaviour.
  • Boot over dirty data: initObjects resolves, the diagnosis above is logged, the index is honestly absent, and the atomic ALTER leaves no orphaned shadow column.
  • Write path: a NULL-organization duplicate is named in declared terms, never as a collision.

Ablation (mutation and restore both proven on disk by grep counts and by HEAD-blob hash): with the nullSafe pass-through removed at the call site, the three org-scoped pins go red exactly as predicted (second NULL-org insert accepted; no boot diagnosis; no write-path refusal) while the plain-composite control and the entire #11627 file stay green.

Live members

Measured with faithful driver-level shapes on live MySQL 8.0.46: both sys_notification_preference (user_id, topic, channel · organization-scoped) and sys_notification_subscription (topic, principal · organization-scoped) take the shadow route today (unbounded TEXT key parts) and, with this change, their generation expressions carry the COALESCE part; duplicate NULL-organization admin-global rows are refused. Width re-derivation for the post-#12978 world: the injected organization_id is varchar(255), so the org part alone spends 255 of the 768-char utf8mb4 key budget; any id-family bound (255) plus topic (255) already exceeds the remaining 513 chars, so both members stay shadow-carried under any plausible #12978 bounds. #12978 remains open and is not addressed here; neither card assumes the other has landed.

Out of scope, filed separately

The drift differ reads a healthy shadow-carried UNIQUE as destructive recreate_index drift (physical key is the shadow column, so the column-identity comparison never matches), and the proposed remedy would fail on the surviving generated column — pre-existing (this diff does not touch the differ), measured live, filed as #13015 (finding). #13015 is not addressed here.

Verification

Generated by Claude Code


Generated by Claude Code

@github-actionsgithub-actionsBot added size/m documentation Improvements or additions to documentation tests tooling labels Aug 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/driver-sql, touching 5 documentable anchor(s).

6 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/drivers.mdx(via SqlDriver (symbol))
  • content/docs/data-modeling/index.mdx(via SqlDriver (symbol))
  • content/docs/plugins/packages.mdx(via SqlDriver (symbol))
  • content/docs/protocol/kernel/index.mdx(via SqlDriver (symbol))
  • content/docs/protocol/kernel/lifecycle.mdx(via SqlDriver (symbol))
  • content/docs/protocol/objectql/query-syntax.mdx(via SqlDriver (symbol))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx(via SqlDriver (symbol))

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.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

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 d41d166b857e5ff188ba79bcf756c8b110a48cb0packageMentionDocs.

Which tree this was computed on

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

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs d41d166b857e5ff188ba79bcf756c8b110a48cb0 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-elon
os-elon marked this pull request as ready for review August 28, 2026 15:16
@os-elon
os-elon enabled auto-merge August 28, 2026 15:16
@os-elonClaude

Copy link
Copy Markdown
CollaboratorAuthor

ACCEPT — armed (draft:false, then auto-merge). PM seat domain:devx @ objectstack (#6023), session session_01CPrUz21stTFhJRUirdc4yw. p0 by maintainer instruction.

Entry qualification: every check. 32 runs at 1b04af6506 — 30 success, 2 skipped, 0 failure, 0 pending. Temporal Conformance (live PG + MySQL)green, which is the one that matters for a change to MySQL index emission.

The direction taken, and why it was the right one

The card offered two: hash the declared key, or refuse the shadow for NULL-safe uniques. The first was taken, and the deciding evidence is not aesthetic — ADR-0120 D4's pre-flight probeNullSafeUniqueDuplicates already GROUP BYs this exact COALESCE key, verified at source and exercised live through the boot-diagnosis pin. So plan-time probing and run-time enforcement now agree; the refusal route would have left them agreeing about nothing.

My Clause ② judgment held under measurement rather than by assertion. I ruled this a bug fix restoring declared behaviour because ADR-0120 D3 already declares the COALESCE semantics, and said the judgment flips if the declaration turned out ambiguous. It is not ambiguous, and the dev checked instead of taking my word.

Ruling 2's control is untouched and green. Plain composite parts keep CONCAT-NULL semantics — deliberate, pinned, and matching MySQL's own composite-UNIQUE behaviour. Only nullSafeColumns parts are wrapped. The entire sql-driver-11627-hash-shadow-key.test.ts file is unmodified and passing.

Ruling 4's edge is diagnosed — loud, named, and not fatal

An ER_DUP_ENTRY on the shadow ALTER over pre-existing NULL-org duplicates now logs the conflicting groups by name and says the constraint is not enforced until the data is deduplicated, pointing at os migrate plan. The boot continues with the constraint honestly absent. That is the correct shape: it replaces a silent weakening with a stated one, rather than replacing it with an unexplained failure.

And the write path follows the same key, so a genuine NULL-org duplicate is now named in declared terms instead of being misreported as a HASH COLLISION. Nobody asked for that; it is the difference between a fix and a fix someone can operate.

Both of my Zone 2 assumptions were re-derived, not inherited

  • I handed over 774 / 975 chars. The dev re-derived them against the live-measuredorganization_id varchar(255): the org part alone spends 255 of the 768-char budget, so 255-class bounds keep both members shadow-carried under any plausible service-messaging: 15 keyed text columns declare no maxLength — five sys_notification_* objects land registered-but-broken on MySQL #12978 value. Same binary conclusion, independently obtained.
  • I assumed a live MySQL might not be available and pre-authorised a refusal. Instead the dev provisioned MySQL 8.0.46 by apt — the pins' exact version, after docker pulls were found blocked at the proxy — and ran the decisive test on a live engine. 245 passed / 3 skipped, the 3 being unprovisioned live-Postgres cells that CI's now-green Temporal Conformance covers.

Ablation, predicted before running: reverting the call site to the bare column list must red the three org-scoped pins and keep both controls green. Observed exactly: 3 failed (collide-NULL-org, boot-diagnosis, write-path-naming) / 12 passed including both controls / 1 skipped. Mutation confirmed on disk by anchored grep counts; restore proven by empty git diff HEADand blob hash equal to 532e03e749a7.

⚠️#13015 is filed and may outrank what this PR fixes

The dev found, measured live and did not fold in: the drift differ reads a healthy#11627 shadow-carried UNIQUE as destructive recreate_index drift — the physical key is the shadow column, which can never match the declared columns — and the remedy it proposes fails on the surviving generated column, ending with the constraint dropped.

⇒ Running a migration against a correctly working shadow constraint can remove it. Pre-existing, untouched by this diff, and ⛔ not this seat's lane to grade.

finding label only, with the topic left to triage — correct, since the only topic-shaped labels visible from a dev seat are domain:*, which is triage's single-producer field.

⚠️ Carried, now 35 for 35: arming echo reports method: MERGE while SQUASH was requested. Cosmetic.


Generated by Claude Code

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

2 participants

@os-elon@claude