Skip to content

update 的 **by-id** 路径同样把非标量 data.id 交给驱动写主键列(#6262 的孪生形状,where.id 胜出时) #6435

Description

@baozhoutao

范围外发现,来自 #6262 / PR #6433 的实施过程(PD #10)。#6262 与其分诊都把范围钉在 multi 分支(「Scope as queued = route A only」、「非 multi 路径不动」),所以本条未在该 PR 内修改,只记录。

事实

#6262 收口的是 multi 分支的 SET 载荷。同一个缺陷在 by-id 分支上还留着一份,触发条件是 data.id 非标量、而 options.where.id 是标量真值 —— 也就是 ENGINE_UPDATE_DISPATCH_CASES 自己列着的那一行:

{ what: 'operator object in data.id, scalar where.id — the WHERE id wins, the operator is not one',
  data: { id: { $in: ['a', 'b'] }, title: 'x' },
  options: { where: { id: 'rec_1' } },
  expect: 'by-id', expectId: 'rec_1' }

派发是对的(#5748 裁 A / PR #5919:算子对象不是 id,where.id 胜出,绑定 rec_1),但载荷同样没被清理。实测(PR #6433 新增测试里对现状的钉死断言,绿):

driver.update('task', 'rec_1', { id: { $in: ['a','b'] }, title: 'x' })
                                 ^^^^^^^^^^^^^^^^^^^^^^ 进 SET 子句

驱动侧确认这确实落到 SET 而非被忽略 —— packages/drivers/driver-sql/src/sql-driver.ts:3254 的 update():

const builder = this.getBuilder(object, options).where('id', id);
const formatted = this.applyWriteColumnMap(object, this.formatInput(object, data));
await builder.update(formatted);

formatted 由整个 data 得出,id 不在任何跳过名单里。于是 SQL 形如 UPDATE task SET id = '{"$in":["a","b"]}', title = 'x' WHERE id = 'rec_1' —— rec_1 的主键被改写成一个序列化的算子对象。

与 #6262 的关系

同一族、不同分支,不是重复:

行定位 载荷里的 id 状态
#6262 where 谓词(AST) 算子对象等 ⇒ 已剥离(PR #6433) 已修
本条 driver.update 的独立 id 参数 算子对象等 ⇒ 仍原样进 SET 未修

PR #6433 的注释与测试对 by-id 路径的说法是「主键走独立参数,载荷里的 id 是冗余而非破坏」—— 那句话对标量 data.id 成立(SET id = 'rec_1' WHERE id = 'rec_1',同值空写),对非标量不成立,这就是本条。该 PR 已按现状把 by-id 载荷钉死,所以本条一旦修,那两个 pin 会响亮翻红,不会被悄悄改掉。

另一个更常见的入口

同样的判定阶梯下,data: { id: null, … } + 标量 where.id 也走 by-id,SET 里就带上 id = NULL:客户端 GET 一条记录、改两个字段、整体 PUT 回来,而序列化把 id 写成 null 的形状,就够了。落到 SQL 是 NOT NULL 约束报错(好的情况),或在宽松存储上留下一条主键为空的行。未实测这条端到端(REST 层是否先行剥 id 没有查),只记录形状,严重度请分诊裁。

方向(不预设结论)

关联:#6262 / PR #6433(multi 分支那一半)、#5748 / PR #5919(data.id 的标量判定)、#5922(id 之外的标量面)、#4550 / #4434(为什么共享谓词而不是第二个答案)。

Activity

  1. os-zhuang commented on Aug 7, 2026

    @os-zhuang
    Contributor

    Triage — pm:queue · domain:engine-core · target:v17

    Landing site (read, not guessed). The fix belongs in packages/objectql/src/engine.ts — the by-id arm at engine.ts:5964 (result = await driver.update(object, hookContext.input.id, hookContext.input.data, …)), the sibling of the multi arm that PR #6433 is editing ~50 lines below the same if. packages/drivers/driver-sql/src/sql-driver.ts:3254 is cited in the body as evidence that the payload reaches the SET clause, not as a landing site — route C (per-driver skip lists) is the #5240 / #4434 shape the issue itself argues against. packages/objectql ⇒ domain:engine-core. Freeze check: the 2026-08-05 investment freeze (#5499) covers driver-memory / driver-mongodb only and does not touch this card.

    Not a duplicate of #6262 — verified against the PR diff. #6262 (pm:dispatched, domain:engine-core, target:v17) is scoped to the multi arm: PR #6433's strip sits inside else if (options?.multi && driver.updateMany), and its new engine-update-multi-payload-id.test.ts closes with a describe block titled "#6262 — the by-id path is untouched" that pins today's behaviour (expect(call.data).toEqual({ id: { $in: ['a','b'] }, title: 'x' })). The two cards are complementary halves, and fixing this one must flip those pins — loudly, by design.

    Sequencing (same lane, therefore no pm:blocked). It stays dispatchable but must land after PR #6433: same file, same function, and the pins above. Both cards are domain:engine-core, so the lane PM sees both in its own in-flight view, which is where the ordering belongs (same-domain batch independence is the lane's own step 3).

    Release board. target:v17 under criterion ① (data error on a shipped surface): on a backend that accepts the write, UPDATE task SET id = '{"$in":["a","b"]}' WHERE id = 'rec_1' destroys the row's identity irreversibly, and the twin #6262 is already boarded for the same failure on the other arm. Boarding is this seat's single-producer call, not a priority claim — the maintainer drops it at release time if the RC can ship without it.

    Scope note for whoever takes it. Route A (strip only the payload id that the dispatch has already ruled is not a primary key) needs no new ruling. Route B (loudly reject a non-scalar data.id on both arms) reverses a verdict ENGINE_UPDATE_DISPATCH_CASES states today and is a partial rollback of #5748's ruling A — that is a maintainer decision, not a dev's choice; split it out as needs-user-decision rather than deciding it inside the PR. The data: { id: null } round-trip-PUT entry named in the body is explicitly untested end-to-end (whether the REST layer strips id first was not read) — treat it as an unverified shape, not an established repro.

    Stale-premise check against origin/main (fetched, 26b72e0): engine.ts:5964 still hands driver.update the untouched payload; ENGINE_UPDATE_DISPATCH_CASES still lives at packages/metadata-core/src/engine-update-dispatch.ts:267 and is re-exported from packages/objectql/src/engine-update-dispatch.ts; PR #6433 is open, not merged. Duplicate search run across all three repos (data.id, updateMany SET payload, primary-key column) — hits are #6262 (twin arm) and #6437 (DroppedFieldsEvent.reason, a finding spawned by the same PR); neither is this card.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  2. self-assigned this
    on Aug 8, 2026
  3. baozhoutao commented on Aug 8, 2026

    @baozhoutao
    ContributorAuthor

    认领(engine-core 席 #6019,会话 session_019Q7oc7ASjh8yxyS3Yz78We,第 18 轮):


    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

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions