diff --git a/.changeset/engine-update-by-id-payload-id-strip.md b/.changeset/engine-update-by-id-payload-id-strip.md new file mode 100644 index 0000000000..294e871dfa --- /dev/null +++ b/.changeset/engine-update-by-id-payload-id-strip.md @@ -0,0 +1,23 @@ +--- +'@objectstack/objectql': patch +--- + +by-id 更新的 SET 载荷不再携带「已被判定不是主键」的 `id`——算子对象 / 数组 / `null` / 假值标量不会再覆盖被更新那一行的主键列 + +`update(o, { id: { $in: ['a','b'] }, title: 'x' }, { where: { id: 'rec_1' } })` 的**派发**自 #5748 裁 A / PR #5919 起就是对的:算子对象不是主键,判定顺着阶梯落到 `where.id`,绑定 `rec_1`(`ENGINE_UPDATE_DISPATCH_CASES` 里就写着这一行,`expect: 'by-id'` / `expectId: 'rec_1'`)。#6262 / PR #6433 收口的是 **multi 臂**的载荷;**by-id 臂**的同一半一直没做。实测(origin/main,记录型 driver 驱动真实引擎): + +``` +driver.update('task', 'rec_1', { "id": { "$in": ["a","b"] }, "title": "x" }) + ^^^^^^^^^^^^^^^^^^^^^^^^ 这是 SET 子句 +``` + +`driver-sql` 的 `update()` 用**整个** `data` 出 `formatted`(`applyWriteColumnMap(formatInput(object, data))`,`id` 不在任何跳过名单里),于是 SQL 形如 `UPDATE task SET id = '{"$in":["a","b"]}', title = 'x' WHERE id = 'rec_1'` —— rec_1 的行标识被一个序列化的算子对象不可逆地覆盖。 + +修法与 #6262 同构:**剥离**,而且只剥「派发已经裁定不是主键」的那一份。成员资格不在这里重新推导,而是**调用派发本身**去问(`resolveEngineUpdateDispatch(data, undefined)` 为 `by-id` 当且仅当 `data.id` 是真值标量)——`asScalarId` 是**故意不导出**的,"给同一个问题添第三种公开写法,正是一条规则长出第二条的方式"。 + +- **零 verdict 变更**:`ENGINE_UPDATE_DISPATCH_CASES` 一行未动,同一个调用仍派发 `by-id`、仍绑 `rec_1`,`engine-update-dispatch.test.ts` 全绿。响亮拒收(路线 B)要反转这条 case,属对 #5748 裁 A 的部分回退,需要新裁决,不在本次范围。 +- **标量 `data.id` 刻意不动**:那里载荷的 `id` **就是**被绑定的主键(标量 `data.id` 压过 `where` 与 `multi`),写出来是 `SET id = 'rec_1' WHERE id = 'rec_1'`,同值空写,冗余而非破坏,且是长期行为;要不要一并剥是另一个决定,已按现状钉死(对照 pin)。 +- **`data: { id: null }` 的回写入口是可达的**(静态读取,非端到端 HTTP 复现):REST 的 `PATCH /data/:object/:id` 只剥 `expectedVersion`,`UpdateDataRequestSchema` 把 `data` 声明为 `z.record(z.string(), z.unknown())`(接受 `null`),协议层 `updateData` 再把请求体**原样**交给 `engine.update(object, data, { where: { id } })`。客户端 GET 一条记录、改两个字段、整体 PUT 回来而序列化把 `id` 写成 `null`,就落在这里。 +- **假值标量同判**:`{ id: 0 }` / `{ id: '' }` 的**判定语义**按 #5747 / #5748 原样不变(仍绑 `where.id`),载荷同样剥离——只剥算子对象而留下假值标量,等于对同一个事实立第二条规则。 + +被剥离时按 `warn` 记一条日志,点明后果与两种正确写法。与 #6262 同样刻意**不**走 `onFieldsDropped`:`DroppedFieldsEvent.reason` 是 `readonly` / `readonly_when` 两值的闭合枚举(#3407 / #3042),扩这个词表是 `packages/spec` 的改动、另有消费者,已单独记为 #6437。 diff --git a/packages/objectql/src/engine-update-by-id-payload-id.test.ts b/packages/objectql/src/engine-update-by-id-payload-id.test.ts new file mode 100644 index 0000000000..456b9627ca --- /dev/null +++ b/packages/objectql/src/engine-update-by-id-payload-id.test.ts @@ -0,0 +1,296 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// objectstack#6435 — the BY-ID half of #6262. A single-row update must not hand +// the driver a SET payload whose `id` the dispatch has already ruled is not a +// primary key. +// +// ## The shape +// +// `update(o, { id: { $in: ['a','b'] }, title: 'x' }, { where: { id: 'rec_1' } })` +// dispatches correctly and has since #5748 / PR #5919: the operator object is +// not an id, so it stops shadowing the ladder, the decision falls through to +// `where.id`, and `rec_1` is bound. `ENGINE_UPDATE_DISPATCH_CASES` states that +// row verbatim (`expect: 'by-id'`, `expectId: 'rec_1'`). What #5748 did NOT do +// — and what PR #6433 fixed for the `multi` arm only — is clean the PAYLOAD. +// Measured on `origin/main` with a recording driver, this branch: +// +// ``` +// driver.update('task', 'rec_1', { id: { $in: ['a','b'] }, title: 'x' }) +// ^^^^^^^^^^^^^^^^^^^^^^ the SET clause +// ``` +// +// `packages/drivers/driver-sql/src/sql-driver.ts`'s `update()` formats the +// WHOLE payload (`applyWriteColumnMap(formatInput(object, data))`; `id` is on +// no skip list), so the row lands as +// `UPDATE task SET id = '{"$in":["a","b"]}', title = 'x' WHERE id = 'rec_1'` — +// rec_1's identity overwritten with a serialized operator object, irreversibly, +// on any backend that accepts the write. +// +// ## Why a strip, and why exactly this wide +// +// Route A, the same answer #6262 gave one arm over: a value the engine has +// ALREADY RULED is not a primary key does not get to sit in the primary-key +// column either. It changes NO verdict — every case in +// `ENGINE_UPDATE_DISPATCH_CASES` still resolves as it did, which is why every +// test below asserts the dispatch alongside the payload +// (`assertEngineUpdateDispatch`, the producer's own decision). +// +// A TRUTHY SCALAR `data.id` is deliberately left alone. There the payload's +// `id` IS the bound key (a scalar `data.id` outranks `where` and `multi` +// alike), so the write is `SET id = 'rec_1' WHERE id = 'rec_1'` — a same-value +// no-op, redundant rather than damaging, and long-standing behaviour. Widening +// the strip over it is a separate decision and is NOT taken here; the contrast +// pins below record that it stayed put. +// +// Route B (reject a non-scalar `data.id` outright) would reverse the +// `expect: 'by-id'` verdict the case-set states today — a partial rollback of +// #5748's ruling A, i.e. a maintainer decision, not this fix. Route C +// (per-driver skip lists) is the #5240 / #4434 shape of five backends giving +// one question five answers. +// +// ## The membership rule, asked and never re-derived +// +// "Is this payload `id` a primary key?" is asked by CALLING the dispatch on the +// payload with no options — `resolveEngineUpdateDispatch(data, undefined)` is +// `by-id` exactly when `data.id` is a truthy scalar. The scalar test itself +// (`asScalarId`) is deliberately unexported: "adding a third public spelling of +// the same question is how a rule with one definition grows a second one" +// (`engine-update-dispatch.ts`). So the strip set is defined by the producer, +// not mirrored beside it, and the table below is a MEASUREMENT of that set +// rather than a second statement of it. + +import { describe, it, expect } from 'vitest'; +import type { EngineUpdateOptions } from '@objectstack/spec/data'; +import { ObjectQL } from './engine.js'; +import { assertEngineUpdateDispatch } from './engine-update-dispatch.js'; + +interface RecordedCall { + readonly fn: 'update' | 'updateMany'; + readonly id?: unknown; + readonly ast?: unknown; + /** A COPY — the engine may keep mutating its own payload after the call. */ + readonly data: Record; +} + +/** + * Records the exact SET payload each driver entry point received. + * + * Same double as `engine-update-multi-payload-id.test.ts` uses, on purpose: + * the two halves of one defect are measured with one instrument. + */ +function makeRecordingDriver() { + const calls: RecordedCall[] = []; + const driver: any = { + name: 'recording', + version: '0.0.0', + supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find() { return []; }, + async findOne() { return null; }, + async create(_o: string, data: Record) { return { id: 'r1', ...data }; }, + async update(_o: string, id: string, data: Record) { + calls.push({ fn: 'update', id, data: { ...data } }); + return { id, ...data }; + }, + async updateMany(_o: string, ast: unknown, data: Record) { + calls.push({ fn: 'updateMany', ast, data: { ...data } }); + return 2; + }, + async delete() { return true; }, + async deleteMany() { return 0; }, + async count() { return 0; }, + async bulkCreate() { return []; }, async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, calls }; +} + +async function makeEngine() { + const engine = new ObjectQL(); + const { driver, calls } = makeRecordingDriver(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject({ + name: 'task', + fields: { title: { type: 'text' }, tenant: { type: 'text' } }, + } as any); + return { engine, calls }; +} + +/** + * Drive the REAL engine and return the one driver call it made — after + * asserting, through the producer's own decision function, that this call + * dispatches where the test says it does. Both halves matter: the whole point + * of #6435 is that the DISPATCH was already right and only the PAYLOAD was not, + * so a test that only looked at the payload could not tell a fixed strip from a + * silently re-routed call. + */ +async function observeWrite( + data: Record, + options: EngineUpdateOptions | undefined, + expect_: { fn: 'update' | 'updateMany'; boundId?: unknown }, +): Promise { + const dispatch = assertEngineUpdateDispatch(data, options); + expect(dispatch.kind, 'dispatch verdict').toBe(expect_.fn === 'update' ? 'by-id' : 'multi'); + if (dispatch.kind === 'by-id') expect(dispatch.id, 'bound primary key').toEqual(expect_.boundId); + + const { engine, calls } = await makeEngine(); + await engine.update('task', data, options); + expect(calls.map((c) => c.fn), 'driver entry points reached').toEqual([expect_.fn]); + if (expect_.fn === 'update') expect(calls[0].id, 'id argument').toEqual(expect_.boundId); + return calls[0]; +} + +/** Own-property, never `in`: `Object.prototype` has no `id`, but say what we mean. */ +function hasIdKey(payload: Record): boolean { + return Object.prototype.hasOwnProperty.call(payload, 'id'); +} + +describe('#6435 — a by-id update strips a ruled-not-an-id `data.id` from the SET payload', () => { + it('the PROBE shape: operator-object data.id + scalar where.id — dispatch still binds rec_1, the payload no longer carries the operator', async () => { + const call = await observeWrite( + { id: { $in: ['a', 'b'] }, title: 'x' }, + { where: { id: 'rec_1' } }, + { fn: 'update', boundId: 'rec_1' }, + ); + // The regression itself: before the fix this payload was + // `{ id: { $in: ['a','b'] }, title: 'x' }` and driver-sql wrote the + // serialized operator object into rec_1's primary-key column. + expect(hasIdKey(call.data), `SET payload was ${JSON.stringify(call.data)}`).toBe(false); + // ...and the strip takes ONLY `id` — the column the caller actually meant + // to write still lands, unchanged. + expect(call.data).toEqual({ title: 'x' }); + }); + + it('array data.id + scalar where.id — same strip, same surviving columns', async () => { + const call = await observeWrite( + { id: ['a', 'b'], title: 'x' }, + { where: { id: 'rec_1' } }, + { fn: 'update', boundId: 'rec_1' }, + ); + expect(hasIdKey(call.data)).toBe(false); + expect(call.data).toEqual({ title: 'x' }); + }); + + it('null data.id + scalar where.id — stripped, not written as a NULL primary key', async () => { + // The entry point #6435's body names as the more common one, and the + // dispatch ladder puts it squarely in the strip set: `asScalarId(null)` is + // `undefined`, so the decision falls through to `where.id` and binds THAT — + // the payload's `null` is a ruled-not-an-id value like any other. + // + // Reachability, read from source rather than asserted: the REST PATCH route + // (`packages/rest/src/rest-server.ts`, `PATCH /data/:object/:id`) strips + // only `expectedVersion` from the body, and `UpdateDataRequestSchema` + // types `data` as `z.record(z.string(), z.unknown())`, which admits `null`; + // the protocol's `updateData` (`packages/metadata-protocol/src/protocol.ts`) + // then calls `engine.update(object, request.data, { where: { id } })` with + // the body VERBATIM. So a client that GETs a row, edits two fields and PUTs + // the whole document back — with `id` serialized as `null` — lands exactly + // here. That read is a static one; no end-to-end HTTP repro was run. + const call = await observeWrite( + { id: null, title: 'x' }, + { where: { id: 'rec_1' } }, + { fn: 'update', boundId: 'rec_1' }, + ); + expect(hasIdKey(call.data)).toBe(false); + expect(call.data).toEqual({ title: 'x' }); + }); + + it('a by-id update that never carried an id is untouched', async () => { + const call = await observeWrite( + { title: 'x' }, + { where: { id: 'rec_1' } }, + { fn: 'update', boundId: 'rec_1' }, + ); + expect(call.data).toEqual({ title: 'x' }); + }); + + it('does not mutate the payload object the CALLER handed in', async () => { + // The strip copies, like every other strip on this path (#5591 / #6343). + // A caller that reuses its payload object across a loop must see what it + // wrote — and `Object.is` says so too: the engine's payload is a DIFFERENT + // object, not the caller's with a key deleted out from under it. + const { engine } = await makeEngine(); + const callerPayload: Record = { id: { $in: ['a', 'b'] }, title: 'x' }; + const snapshot = callerPayload; + await engine.update('task', callerPayload, { where: { id: 'rec_1' } }); + expect(Object.is(callerPayload, snapshot), 'caller kept its own object').toBe(true); + expect(callerPayload).toEqual({ id: { $in: ['a', 'b'] }, title: 'x' }); + }); +}); + +describe('#6435 — the falsy scalars keep the #5747 / #5748 dispatch semantics', () => { + // NOT a new verdict. `0` and `''` are scalars, so they take the scalar branch + // of the id test and then fail its TRUTHINESS half — the engine branches on + // `if (hookContext.input.id)` and always has (the dispatch module's header + // point 3). So `data: { id: 0 }` beside `where: { id: 'rec_1' }` falls + // through to `where` and binds `rec_1`, before this change and after it. + // + // What DOES change is the payload, on the same argument as the operator + // object: the dispatch has ruled this value is not the primary key of the row + // being written, so `SET id = 0` would replace rec_1's key with `0`. Leaving + // falsy scalars in while stripping operator objects would be a SECOND rule + // about one fact — the shape #4550 / #4434 exist to prevent, and the same + // call PR #6433 made for the multi arm. + for (const falsy of [0, ''] as const) { + it(`data.id = ${JSON.stringify(falsy)} beside a scalar where.id still binds rec_1 (verdict unchanged) and is stripped`, async () => { + const call = await observeWrite( + { id: falsy, title: 'x' }, + { where: { id: 'rec_1' } }, + { fn: 'update', boundId: 'rec_1' }, + ); + expect(hasIdKey(call.data)).toBe(false); + expect(call.data).toEqual({ title: 'x' }); + }); + } +}); + +describe('#6435 — the contrast pins: what this change deliberately does NOT touch', () => { + it('a TRUTHY SCALAR data.id is still handed to the driver as sent', async () => { + // Route A's boundary, pinned. Here the payload's `id` IS the bound key, so + // the write is a same-value no-op rather than an identity rewrite. If a + // later change decides to strip this too, THIS is the assertion that must + // flip — deliberately, with its own reasoning. + const call = await observeWrite( + { id: 'rec_1', title: 'x' }, + { where: { id: 'rec_1' } }, + { fn: 'update', boundId: 'rec_1' }, + ); + expect(call.data).toEqual({ id: 'rec_1', title: 'x' }); + }); + + it('a truthy scalar data.id that DISAGREES with where.id still wins, payload as sent', async () => { + // `ENGINE_UPDATE_DISPATCH_CASES`: "a SCALAR data.id still wins over a + // scalar where.id" ⇒ bound id `rec_1`, not `rec_2`. Unchanged here. + const call = await observeWrite( + { id: 'rec_1', title: 'x' }, + { where: { id: 'rec_2' } }, + { fn: 'update', boundId: 'rec_1' }, + ); + expect(call.data).toEqual({ id: 'rec_1', title: 'x' }); + }); + + it('the MULTI arm is exactly as PR #6433 left it', async () => { + // The sibling half, re-measured from this file so a regression in either + // arm shows up next to the other. #6433's own suite is the primary pin. + const call = await observeWrite( + { id: { $in: ['a', 'b'] }, title: 'x' }, + { multi: true }, + { fn: 'updateMany' }, + ); + expect(hasIdKey(call.data)).toBe(false); + expect(call.data).toEqual({ title: 'x' }); + expect(call.ast).toEqual({ object: 'task' }); + }); + + it('a multi update selecting rows by an id SET keeps its predicate and its payload', async () => { + const call = await observeWrite( + { title: 'x' }, + { where: { id: { $in: ['a', 'b'] } }, multi: true }, + { fn: 'updateMany' }, + ); + expect(call.data).toEqual({ title: 'x' }); + expect(call.ast).toEqual({ object: 'task', where: { id: { $in: ['a', 'b'] } } }); + }); +}); diff --git a/packages/objectql/src/engine-update-multi-payload-id.test.ts b/packages/objectql/src/engine-update-multi-payload-id.test.ts index 20199af099..e4fe0b6812 100644 --- a/packages/objectql/src/engine-update-multi-payload-id.test.ts +++ b/packages/objectql/src/engine-update-multi-payload-id.test.ts @@ -193,14 +193,24 @@ describe('#6262 — the falsy scalars keep the #5747 / #5748 dispatch semantics' } }); -describe('#6262 — the by-id path is untouched', () => { +describe('#6262 — the by-id path, as this file left it and as #6435 changed it', () => { + // These three pins were written by #6262 (PR #6433) to record the by-id + // arm's behaviour AT THAT TIME, "so a future widening of the strip is a + // deliberate act". #6435 is that deliberate act, and it is a widening of + // exactly ONE of the three: the last one, whose payload `id` the dispatch + // had already ruled is not a primary key. The first two are unchanged and + // stay here as the contrast — a SCALAR `data.id` is still handed to the + // driver as sent. Full by-id coverage lives in its own file next door, + // `engine-update-by-id-payload-id.test.ts`. + it('a scalar data.id outranks multi:true and reaches driver.update with the payload AS SENT', async () => { const call = await observeWrite({ id: 'rec_1', title: 'x' }, { multi: true }, 'update'); expect(call.id).toBe('rec_1'); - // The by-id branch has always handed the driver the payload including - // `id`, and #6262 is scoped to the multi branch: `driver.update` is given - // the primary key SEPARATELY, so the key in the payload is redundant, not - // damaging. Pinned so a future widening of the strip is a deliberate act. + // UNCHANGED by #6435, and deliberately so: here the payload's `id` IS the + // bound primary key (a scalar `data.id` outranks `where` and `multi` + // alike), so the write is `SET id = 'rec_1' WHERE id = 'rec_1'` — a + // same-value no-op, redundant rather than damaging. Widening the strip + // over this shape is a separate decision (#6435's scope note). expect(call.data).toEqual({ id: 'rec_1', title: 'x' }); }); @@ -210,18 +220,22 @@ describe('#6262 — the by-id path is untouched', () => { expect(call.data).toEqual({ title: 'x' }); }); - it('operator data.id BESIDE a scalar where.id: the where id wins, and the operator does not reach the payload column', async () => { - // #5748's headline shape — verdict `by-id`, bound id `rec_1`. The payload - // still carries the operator object here, because this is the by-id branch - // and the primary key travels in its own argument; the row's identity is - // never taken from the payload. What #6262 fixes is only the branch where - // the payload IS the SET clause. + it('[#6435] operator data.id BESIDE a scalar where.id: the where id wins, and the operator no longer reaches the SET clause', async () => { + // #5748's headline shape — verdict `by-id`, bound id `rec_1`. This pin + // FLIPPED at #6435, by design: the assertion used to read + // `toEqual({ id: { $in: ['a','b'] }, title: 'x' })`, recording that the + // by-id arm handed the operator object straight to `driver.update`'s data + // argument. `driver-sql` formats that whole payload into the SET clause, + // so `rec_1`'s primary key was rewritten to `'{"$in":["a","b"]}'`. + // + // The dispatch is untouched — same verdict, same bound id — and only the + // payload changed, which is why the two assertions below sit side by side. const call = await observeWrite( { id: { $in: ['a', 'b'] }, title: 'x' }, { where: { id: 'rec_1' } }, 'update', ); expect(call.id).toBe('rec_1'); - expect(call.data).toEqual({ id: { $in: ['a', 'b'] }, title: 'x' }); + expect(call.data).toEqual({ title: 'x' }); }); }); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 0a4bca4fdc..6f54286c5c 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -5970,6 +5970,81 @@ export class ObjectQL implements IObjectQLEngine { // terms, so it owes the same counterexample. const onAdmittedValueShapeViolation = this.admittedViolationSink(object); if (hookContext.input.id) { + // [#6435] The by-id half of #6262's strip — same defect, other + // arm. Reaching this branch means the dispatch found a truthy + // scalar id, but NOT necessarily in the payload: when `data.id` + // is a non-scalar (an operator object, an array, `null`) or a + // falsy scalar, `resolveEngineUpdateDispatch` rules it is not a + // primary key and falls through to `options.where.id`, binding + // THAT (#5748 / PR #5919 — `ENGINE_UPDATE_DISPATCH_CASES` states + // it: `operator object in data.id, scalar where.id` ⇒ `by-id`, + // `expectId: 'rec_1'`). The dispatch is right; the PAYLOAD was + // never cleaned. Measured on origin/main, this very branch: + // + // driver.update('task', 'rec_1', { id: { $in: ['a','b'] }, title: 'x' }) + // ^^^^^^^^^^^^^^^^^^^^^^ the SET clause + // + // `driver-sql`'s `update()` formats the WHOLE payload + // (`sql-driver.ts`, `applyWriteColumnMap(formatInput(object, + // data))` — `id` is on no skip list), so the row is written as + // `UPDATE task SET id = '{"$in":["a","b"]}', title = 'x' WHERE + // id = 'rec_1'`: rec_1's identity is overwritten with a + // serialized operator object, irreversibly, on any backend that + // accepts it. + // + // The rule is #6262's, unchanged and asked of the payload rather + // than of the branch: a value the engine has ALREADY RULED is + // not a primary key does not get to sit in the primary-key + // column. It is asked by CALLING the dispatch — "would this + // payload, on its own, identify a row?" — never by re-deriving + // the scalar test here: `asScalarId` is deliberately unexported + // (`engine-update-dispatch.ts`: "adding a third public spelling + // of the same question is how a rule with one definition grows a + // second one"), and a hand-mirrored copy is the #4434 / #4550 + // failure this family exists to prevent. + // + // Deliberately NARROW, and the narrowness is the whole scope: + // + // - A TRUTHY SCALAR `data.id` is left exactly as it is. There + // the payload's `id` IS the bound key (it outranks `where` — + // same case-set), so the write is `SET id = 'rec_1' WHERE id + // = 'rec_1'`: a same-value no-op, redundant rather than + // damaging, and long-standing behaviour. Widening the strip + // to cover it is a separate decision, not a rider here. + // - Rejecting the call instead (#6435's route B) would reverse + // the `expect: 'by-id'` verdict the case-set states today — + // a partial rollback of #5748's ruling A, i.e. a maintainer + // decision. This change alters NO verdict: the same call + // still dispatches by-id and still binds `rec_1`. + // - Per-driver skip lists (route C) are the #5240 / #4434 shape + // of five backends answering one question five ways. + // + // Same choice as #6262 on the reporting seam: NOT routed through + // `reportDroppedFields`, because `DroppedFieldsEvent.reason` is a + // closed enum over the two READ-ONLY strips (`readonly` / + // `readonly_when`, #3407/#3042) and this drop is neither; + // widening that vocabulary is a `packages/spec` change with its + // own consumers (filed separately as #6437). The `warn` is the + // #4632 duty meanwhile — the caller is told the write succeeded. + const preIdById = hookContext.input.data as Record | null | undefined; + if ( + preIdById && + typeof preIdById === 'object' && + Object.prototype.hasOwnProperty.call(preIdById, 'id') && + resolveEngineUpdateDispatch(preIdById as EngineUpdateDispatchData, undefined).kind !== 'by-id' + ) { + const { id: notAnId, ...withoutId } = preIdById; + hookContext.input.data = withoutId as any; + this.logger.warn( + `Update on '${object}' of record ${String(hookContext.input.id)}: dropped 'id' from the ` + + `write payload. The row is identified by the id argument, and the engine has already ruled ` + + `this payload value is not a primary key (${JSON.stringify(notAnId) ?? String(notAnId)}) — ` + + `writing it would have overwritten that row's primary-key column. To update ONE row by id, ` + + `pass a SCALAR id (\`update(object, { id, ...fields })\` or \`{ where: { id } }\`); to ` + + `SELECT rows by an id set, put it in \`where\` ` + + `(\`{ where: { id: { $in: [...] } }, multi: true }\`).`, + ); + } await this.encryptSecretFields(object, hookContext.input.data as Record, opCtx.context, hookContext.input.options); normalizeMultiValueFields(updateSchema, hookContext.input.data as Record); validateRecord(updateSchema, hookContext.input.data as Record, 'update', { mediaValueShapeStrict, valueShapeStrict, messages: updateMsgCtx, onAdmittedValueShapeViolation });