Skip to content

drivers(sql): MySQL silently merges an upsert on a unique key the caller never named, and rewrites the merged row's primary key — measured, and it is a wrong write with no error #8621

Description

@os-zhuang

Filed out of #8592, which was dispatched to measure this and is scope-guarded to the measurement only. Step 2 of that card — changing MySQL's accept set — was ruled a minor with its own argument, so here it is. Unassigned.

The measurement (this is now an observation, not an inference)

Live MySQL 8.0.46 (Ubuntu noble mysql-server, mysqld --daemonize, default_time_zone='+08:00'), driven through the same knex + mysql2 path SqlDriver.upsert takes. Table: email is the column named in conflictKeys and has no unique index; tax_id carries the only unique index.

CREATE TABLE `os8592_mismatched` (
  `id` varchar(255) NOT NULL, `email` varchar(255) DEFAULT NULL,
  `tax_id` varchar(255) DEFAULT NULL, `title` varchar(255) DEFAULT NULL,
  PRIMARY KEY (`id`),
  UNIQUE KEY `uniq_os8592_mismatched_tax_id` (`tax_id`)
)
seed  upsert({email:'a@b.com',     tax_id:'T-1', title:'first'},  ['email'])
      -> RESOLVED. rows=[{id:'VBjOQwQp3uTtewte', email:'a@b.com', tax_id:'T-1'}]

B     upsert({email:'other@b.com', tax_id:'T-1', title:'second'}, ['email'])
      -> RESOLVED. rows=[{id:'RnSaXzGO69OKkP_D', email:'other@b.com', tax_id:'T-1'}]
         ONE row: merged on `tax_id`, across two DIFFERENT `email` values.
         And the surviving row's PRIMARY KEY changed.

D     seed, then upsert({email:'a@b.com', tax_id:'T-2'}, ['email'])
      -> RESOLVED. TWO rows, both email='a@b.com'.

Reproduced across two independent runs. The compiled statement is the cause, and #8567 already pinned it with no server: knex compiles onConflict(['email']).merge(...) on mysql2 to ON DUPLICATE KEY UPDATE, which takes no conflict target — the named keys are dropped before the statement leaves the process.

Three consequences, and the middle one is the sharpest

  1. No refusal. The identical call is VALIDATION_ERROR / 400 on SQLite (drivers(sql): an upsert whose conflictKeys have no backing unique index throws a raw SqliteError — the local twin of #8413's consequence 2 #8445) and Postgres (drivers(sql): the unbacked-conflict-target refusal is SQLite-only — Postgres and MySQL still answer the raw driver error #8567). On MySQL it resolves.
  2. The merged row's primary key is replaced. id sits in the merge set, so the statement carries id = values(id) and the stored row's identity is overwritten with the fresh nanoid minted for the insert that lost. Every external reference to the old id dangles, and nothing reports an error. (This is the general form of a defect filed separately — it is not MySQL-specific.)
  3. The named key is not honoured at all. Two rows with the same email coexist after a caller explicitly asked to merge on email.

So MySQL fails in both directions at once: it merges where the other two dialects refuse, and it does not merge where it was told to.

What this card has to decide

The behaviour is pinned as a characterization in sql-driver-upsert-conflict-target-dialects.test.ts (landing with #8592), loudly marked as recording a defect. Those pins go red when this card lands and must be rewritten to the refusal, not relaxed.

Options, for the argument this deserves:

Recommendation is A, on the AI-authored-metadata axis in particular: a mismatched conflictKeys is exactly the kind of error a generated metadata app produces, and a consumer-side silent merge is where it hides and multiplies. The driver already knows the declared indexes — the check belongs at the producer, not as a tolerant parse at the consumer.

Related: #8592 (the measurement), #8567 (compiled SQL read, Postgres measured), #8445 / #8413 (the two faces that refuse), #5240 (one condition, one wording).

Activity

  1. added theissue type on Aug 14, 2026
  2. hotlong commented on Aug 14, 2026

    @hotlong
    Contributor

    Blocked-by: #8592

    Triage: lands in packages/drivers/driver-sql (pre-flight conflict-target validation in SqlDriver.upsert) ⇒ domain:drivers, type Bug, state pm:blocked on #8592, auto-adjudicated → Option A (pre-flight refusal).

    Adjudication basis (veto window — flagged in the round report):

    Blocked-by rationale: same file surface as #8592's landing tests. Same-file hard serial with #8622 — never batch the two together. Refusal wording per #5240 (one condition, one wording).
    Size/model suggestion: M, mode:subagent, opus (driver accept-set change with a dialect-matrix test).


    Generated by Claude Code

  3. claude commented on Aug 14, 2026

    @claude
    Contributor

    Unlock sweep: blocker cleared. pm:blocked → pm:queue.

    Declared blocker: objectstack#8592 — verified closed against the GitHub API on 2026-08-14. No other blocker is declared on this card.

    ⚠️ This card's Blocked-by: line lives in a comment, not the body. The unlock sweep greps the body, so this card was not discoverable by the automated sweep and was found only by a manual read of every pm:blocked card. Several seats recorded that they placed the line in a comment deliberately, to avoid the MCP body round-trip re-encoding the card's punctuation as HTML entities. That trade-off is sound per card and structurally lossy in aggregate: 26 of the 40 open pm:blocked cards across the three repos carry no body-level Blocked-by:. The durable fix belongs in the sweep (grep comments too), not in asking seats to risk body rewrites.

    Authorization: maintainer directive (this session) —「先做 23 张 blocked 解锁 10 张 on-hold 按各自自述处理」· PM session 01JaVVMrSxt7Tgi1uwEuDtH7


    Generated by Claude Code

  4. self-assigned this
    on Aug 14, 2026
  5. hotlong commented on Aug 14, 2026

    @hotlong
    Contributor

    Claim: PM loop round 3
    Session: session_01XeQRiAa7vYRVX5Fog7Zby8
    Branch: claude/issue-8621-mysql-unbacked-conflict-preflight
    Worktree: objectstack-issue-8621
    Domain: domain:drivers
    File surface: packages/drivers/driver-sql/src/sql-driver.ts (the upsert pre-flight, ahead of statement compilation) + sql-driver-upsert-conflict-target-dialects.test.ts (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: opus
    Serial constraints cleared: the hard serial on #8622 is discharged — PR #8741 merged as a4acb8d, verified by two readings (commit and the artifacts: remoteColumn(object,'id','id') in the insert-only set, the noopMergeColumns fallback, and the MySQL identity pin flipped to preservation). Open-PR branches re-read at claim time (not reused from earlier this round): the live ones are 8231, 8287, 8536, 8566, 8595, 8691 — none touches driver-sql.

    ⚠️ Stale-premise check: this card is now SMALLER than filed, and the change is in the pins

    Measured on origin/main after #8622 landed:

    the card's three consequences state now
    ① no refusal — MySQL resolves where SQLite/PG refuse still open — this is the card
    ② the merged row's primary key is replaced ✅ fixed by #8622, and its pin already asserts preservation
    ③ the named key is not honoured at all still open — this is the card

    The card's own body predicted this: it flagged ② as "the general form of a defect filed separately — it is not MySQL-specific." That separate card landed first, deliberately, so ② is gone from this one's scope.

    ⇒ The instruction «those characterization pins go red when this card lands and must be rewritten to the refusal, not relaxed» now applies to ① and ③ only. ⛔ The identity pin is not yours to move — #8622 already rewrote it to assert preservation, and re-touching it would undo a landed fix. Confirmed present on main as KEEPS the surviving row's primary key, even merging on that wrong key.

    What the adjudication already settled

    auto-adjudicated → option A (pre-flight refusal), on sibling-branch inheritance: SQLite (#8445) and Postgres (#8567/#8413) are already ruled loud VALIDATION_ERROR refusals for an unbacked conflict target, and MySQL is the third branch of that family. ⛔ Option B (document the divergence) is rejected. The fork-back clause stands: a genuine MySQL-specific semantic difference the mother ruling does not cover ⇒ stop and return the card to needs-user-decision, never a silent re-decision.

    One mechanism note the dispatch will carry

    The existing refusal is reactive — sql-driver.ts:5138 catches a server error via isUnbackedConflictTargetError and rethrows refuseUnbackedConflictTarget. On MySQL no error is ever raised, so that catch cannot fire; option A is a genuinely different mechanism (check the declared indexes before compiling). Reusing the same refusal wording per #5240 is the goal; reusing the same code path is not automatically possible.

    Also worth knowing: unbacked-conflict-target.ts's module head was corrected by #8730, which merged as bbbfcfc earlier this round — read it on current main, not from memory.

    Changeset must be minor and say plainly that MySQL calls which previously "resolved" now throw.


    Generated by Claude Code

  6. hotlong commented on Aug 14, 2026

    @hotlong
    Contributor
    {
      "issue": 8621,
      "status": "done",
      "branch": "claude/issue-8621-mysql-unbacked-conflict-preflight",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8761",
      "premise_still_valid": true,
      "summary": "Implemented Option A as ruled: SqlDriver.upsert now consults the table's physical PRIMARY KEY and UNIQUE indexes before compiling on MySQL and refuses an unbacked conflict target with the existing VALIDATION_ERROR/400 wording (#5240). The wording, code and status are reused; the code path could not be, since the reactive catch classifies a server error MySQL never raises. It runs on MySQL alone by design — SQLite and Postgres already refuse from the server and attach the server's own sentence as `cause`, which a pre-flight cannot reconstruct — and it refuses only what it can prove: a failed introspection, a table reporting no keys at all, and a stale cache all proceed (the cache is re-read before any refusal). I raised a live MySQL 8.0.46 and a live Postgres 16.13 in the container, so all three dialect cells were measured here rather than deferred to CI. TWO THINGS THE PM MUST READ. (1) The prompt's `do not touch the identity pin` could not be obeyed literally: #8622's pin measures identity preservation ACROSS a wrong-key merge, and on the mismatched table that call is now refused, so the phenomenon is gone from that fixture and the pin went red by construction. I moved its FIXTURE only — to a table where a wrong-key merge still happens (both columns unique) — leaving its assertion, strength and failure message byte-identical; deleting or relaxing it would have dropped #8622's only MySQL-cell coverage. Its docblock now records that this container executed it on live MySQL for the first time, green, and keeps its standing do-not-relax instruction. (2) The fix closes the unbacked-target hole and NOT the whole MySQL gap: ON DUPLICATE KEY UPDATE carries no target even when the named one IS backed, so a second unique index can still absorb the conflict. Measured, filed as #8755, and pinned in this file as the scope boundary so nobody reads the PR as making MySQL honour conflictKeys. No fork-back: that is a different condition, not a semantic difference blocking the mother ruling.",
      "tests": "Union re-run AFTER the final commit, at HEAD 68d1b2d, clean tree, exit codes read unpiped: check:nul-bytes 0, check:test-source-alias 0, check:type-source-resolution 0, check:query-options-erasure 0, check:type-check-coverage 0, check:type-check-debt 0 (--re-measure, full closure built first via turbo run build --filter=./packages/* --filter=./packages/*/*). Re-derived with scripts/pm/dispatch-gates.mjs against the actual changed paths: exactly the families the dispatch named, no additions. LIVE MATRIX, all three cells provisioned in this container (no un-run cell): MySQL 8.0.46 (Ubuntu noble mysql-server, mysqld --daemonize, default_time_zone='+08:00') and Postgres 16.13 (initdb + pg_ctl, timezone=Asia/Shanghai), TZ=America/New_York. BEFORE, against unmodified main: #8592's transcript reproduced line for line — upsert({email:'a@b.com',tax_id:'T-1'},['email']) resolved, then {email:'other@b.com',tax_id:'T-1'} merged to ONE row on tax_id, and the seed+{tax_id:'T-2'} pair left TWO rows sharing email. AFTER: both refused, code=VALIDATION_ERROR, status=400, same first sentence as the SQLite/Postgres sweep, and the table empty afterwards (the assertion that separates a pre-flight from a post-hoc classification). Controls green on the same live MySQL: a UNIQUE-backed target still merges, the primary key named explicitly is accepted, the default no-conflictKeys path is untouched. Full runs on the merged tree: pnpm --filter @objectstack/driver-sql test -> 100 files / 2096 tests passed; driver-turso (the direct SqlDriver subclass) 37 files / 986; objectql (the one production conflictKeys caller) 206 files / 3629; pnpm --filter @objectstack/driver-sql typecheck clean; pnpm --filter @objectstack/spec check:generated after merging main -> all 13 artifacts up to date. REVERSE VERIFICATION, direction predicted before running and recorded in the test docblock: with the fix committed first and the call site's this.isMysql guard inverted to !this.isMysql, the MySQL pins go red AND the SQLite/Postgres sweep goes red differently — refusal pins stay green while the `cause` pin fails, because the pre-flight answers first with a driver-built cause. Measured: exactly 7 failures, 5 MySQL pins plus one `cause` pin on each of SQLite and Postgres. Restored with git checkout HEAD -- (never git stash). No ablation in this change, so no dist rebuild claim is made. One transient false red worth recording: driver-turso's date-bucket-parity suite failed to resolve @objectstack/verify on the fresh worktree — an unbuilt dependency, green after building the closure, not related to this diff.",
      "open_questions": [
        {
          "question": "The dispatch said do not touch #8622's identity pin, but the ruled fix makes it red by construction — it asserts identity across a merge on a key the caller never named, and that call is now refused on the mismatched table. I moved its fixture (to the both-columns-unique table, where a wrong-key merge still occurs) and left the assertion untouched. Is that the disposition the PM wants?",
          "options": [
            "A — keep it as landed: fixture moved, assertion/strength/message untouched, docblock records why and that this container ran it green on live MySQL",
            "B — delete the MySQL-cell copy and rely on the sweep's SQLite/Postgres identity pins, losing #8622's only MySQL coverage",
            "C — keep it on the mismatched table asserting the refusal instead, which changes what it measures from identity to refusal and silently retires #8622's claim"
          ],
          "recommendation": "A. B and C both end with #8622's identity fix unmeasured on MySQL — exactly the coverage the #8622 docblock said CI would be the first to execute. A is the only option that keeps the claim alive at full strength, and it is now measured rather than carried: this container executed it on live MySQL 8.0.46, green."
        }
      ],
      "out_of_scope_findings": [
        "filed as #8755: on MySQL an upsert still merges on a unique key the caller never named even when the named conflict target IS backed — ON DUPLICATE KEY UPDATE carries no target, so any unique index can absorb the conflict; measured on live MySQL 8.0.46, unlabeled for PM triage, and pinned as the scope boundary in the dialects suite"
      ]
    }

    Generated by Claude Code


    Generated by Claude Code

  7. hotlong commented on Aug 14, 2026

    @hotlong
    Contributor

    ACCEPT — PR #8761.

    Verdict on the two flagged items

    (1) The identity-pin fixture move: option A, as landed. My dispatch instruction was the thing at fault, not the change.

    I wrote "⛔ do not touch the identity pin" into the dispatch. That instruction was literally unobeyable once the ruled fix landed, and I should have seen it before dispatching. #8622's pin measures identity preservation across a wrong-key merge; the ruled pre-flight refuses that call on a table whose named conflict target is unbacked, so on the MISMATCHED fixture the phenomenon under measurement no longer occurs. The pin went red by construction — not because the fix broke identity preservation, but because the fix removed the event the pin was watching.

    I framed this as a two-way choice (obey the pin, or change the fix). There was a third path and I failed to anticipate it: move the fixture, keep the measurement. Moving to WRONG_KEY — where both columns are unique, so the named target passes the pre-flight and a wrong-key merge still genuinely occurs — leaves assertion, strength and failure message byte-identical while restoring the phenomenon. That is a fixture relocation, not a weakening, and it is the correct resolution.

    The reasoning for rejecting B and C is recorded because it is the part worth keeping: both end with #8622's identity fix unmeasured on MySQL — precisely the coverage that #8622's own docblock deferred to CI as first executor. A is the only option that keeps the claim alive at full strength. And it is now measured rather than carried: this container ran it on live MySQL 8.0.46, green. That closes the one cell #8622 left un-run.

    Rule going forward, on me: when a dispatch pins an existing test as untouchable, I must first check whether the ruled fix makes that test's phenomenon unreachable. A pin protects an assertion, never a fixture.

    (2) The #8755 scope boundary: correct, and the in-file pin is the right form.

    ON DUPLICATE KEY UPDATE carries no conflict target at all, so a second unique index can still absorb the conflict even when the named target is backed. Measuring that, filing it as #8755, and pinning the boundary in the test file is exactly right — without it this PR reads as "MySQL now honours conflictKeys", which it does not. It closes the unbacked-target hole only. Keeping the boundary as an executable pin rather than a prose comment means the day someone does close the rest, the pin fails and forces them to update the claim.

    What I verified

    • Placement: the pre-flight sits before the retry loop, therefore before fillAutoNumberFields — a refusal never burns an autonumber. This was the load-bearing constraint and it holds.
    • Fails open on uncertainty: null introspection, zero keys, and a stale-cache re-read before refusing. A pre-flight that guesses "unbacked" and refuses a legitimate write would be strictly worse than the bug; it doesn't.
    • Cache invalidation: this.physicalKeyIndexes.delete(tableName) in initObjects.
    • sys_setting's declared row identity is unenforced on every tenant and global row — user_id is NULL there and SQL UNIQUE is NULL-distinct #8629's pin is safe: sql-driver-sys-setting-organization-unique.test.ts is untouched in the PR's file list.
    • Changeset: driver-sql: minor. Correct — this narrows a previously-accepted input set on MySQL.
    • Reverse verification: direction was predicted before running and predicted non-uniform — MySQL pins red and SQLite/Postgres red differently (refusal pins green, cause pin fails, because the pre-flight answers first with a driver-built cause). Measured exactly that: 7 failures, 5 MySQL plus one cause pin on each of SQLite and Postgres. A non-uniform prediction that lands is much stronger evidence than a uniform one, and it rules out "the guard is inert and the pins pass for unrelated reasons."
    • All three dialect cells measured in-container — live MySQL 8.0.46 and live Postgres 16.13. Nothing rides on CI for first execution.

    One item before merge

    This narrows a shipped accept set, so I'm checking whether content/docs/protocol/objectql/query-syntax.mdx needs a line about MySQL refusing an unbacked conflictKeys target. If it does, it goes in this PR; if the page never made the MySQL claim, no doc change and I'll say so here.


    Generated by Claude Code

  8. hotlong commented on Aug 14, 2026

    @hotlong
    Contributor

    Docs check — resolved: no doc change belongs in this PR.

    Measured on origin/main:

    So the refusal behaviour was never carried in prose. The predecessor cards (#8445, #8567) shipped the SQLite/Postgres refusals with no prose doc line either; there is no page making a MySQL-specific accept claim that this PR narrows, and therefore no doc drift to repair. query-syntax.mdx exists but never mentions conflictKeys — my flag in the ACCEPT was a precaution, and it comes back clean.

    releases/v17.mdx is off-limits to a code PR by repo rule — release notes are compiled centrally at release time from changesets. This PR's driver-sql: minor changeset is the correct and sufficient input; the release process folds #8621 and the #8755 boundary into that existing family line.

    Item closed. Nothing outstanding on this PR except gate conclusions.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions