Skip to content

drivers(sql): an upsert that merges on a non-PK conflict key silently REWRITES the existing row's primary key — measured on SQLite and MySQL alike #8622

Description

@os-zhuang

Found while measuring #8592. Unassigned. Not MySQL-specific, and not about conflict-target validity — this reproduces on a perfectly well-formed upsert whose conflict target IS backed by a unique index.

What was measured

Table with email declared unique: true — the conflict target is properly backed, so this is the supported path, not an error path. Two upserts merging on ['email']:

upsert({email:'x@b.com', title:'first'}, ['email'])
upsert({email:'x@b.com', title:'second'}, ['email'])

Observed — one row throughout, as intended, but watch the id:

[sqlite] before=[{id:'rSxhabt0nVz-9RsL', title:'first'}]
after =[{id:'hXqcSC45F3IXFr5k', title:'second'}] idPreserved=false
[mysql] before=[{id:'euKyrh1dzHE153eC', title:'first'}]
after =[{id:'Ycqn7yHaGHvUXi-c', title:'second'}] idPreserved=false

SQLite (in-process) and live MySQL 8.0.46 both rewrite it. Postgres was not raised for this run, so it is deliberately NOT claimed here — the mechanism below is dialect-independent and predicts the same result, but that is an inference and wants the conformance runner to confirm it rather than a third row in the table above.

Why it happens

SqlDriver.upsert mints an id whenever the caller does not supply one:

}elseif(toUpsert.id===undefined){toUpsert.id=nanoid(DEFAULT_ID_LENGTH);}

and then builds the merge set as everything not insert-only:

constinsertOnlyColumns=this.insertOnlyUpsertColumns(object);constmergeColumns=Object.keys(formatted).filter((c)=>!insertOnlyColumns.has(c));

insertOnlyUpsertColumns covers created_at and auto_number columns (#7011) — not id. So id travels in the merge set:

insert into`t` (…) values (…)
on conflict (`email`) do updateset …, `id`= excluded.`id`, …

When the conflict key IS the primary key (the default ['id'] path) that clause is a no-op — the value is identical, which is why this has stayed invisible. The moment conflictKeys names anything else, the losing insert's freshly minted nanoid overwrites the winning row's identity.

Why it matters

id is the platform's stable row identity: it is what relationships, audit records, external id mappings and any client-held reference point at. A merge-path upsert on a business key is the ordinary way to ingest external data — and today every such call silently re-identifies the row it merged into. No error is raised on any dialect.

This is the same argument #7011 already accepted for auto_number"the number is an externally visible business identifier once assigned" — and the primary key is the strongest instance of it.

Likely fix

Treat the primary key as insert-only on the merge path, i.e. add id to insertOnlyUpsertColumns, exactly as created_at and auto_number already are. Wants checking against the _id alias branch, and against any caller that deliberately re-keys a row (update() is the deliberate path, per #7011's reasoning).

Existing coverage does not catch it: the control in sql-driver-upsert-conflict-target-dialects.test.ts asserts row count and title after a backed merge, never the id.

Related: #7011 (the same exclusion argument, for auto_number), #8592 (where this surfaced), #8621, #8567.

Metadata

Metadata

Assignees

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions