Skip to content

A write's response serialises a Needs-computed column at its zero value — the ADR says the column is left out, and a definite false is the lie the obligation exists to prevent #163

Description

@jryannel

What happens

A resource that opts into a Needs-computed column serialises that column in its create and update responses at its Go zero value — present and definite, not absent.

Reproduced against the rest package's own fake, using its own Starred model (is_starred, Needs: ["viewer"], bind supplied by a registered BeforeQuery hook):

resp := api.Patch("/starred/s1", map[string]any{"title": "New"})
statement sent:  UPDATE "starred" SET "title" = $1 WHERE "id" = $2 RETURNING "id", "title"
response body:   {"id":"s1","title":"New","is_starred":false}

The statement is right: ADR-0041 decided a parameterised expression is left out of a write's RETURNING because a mutation has nowhere to take the bind from, and writeReturning (mutate.go:833) does exactly that. But the response projection is b.selectable, which includes every computed column the mount opted into (rest/item.go:226, :292) — so the scanned struct's zero value goes out on the wire as a real answer.

Why it is a bug and not a caveat

Three texts in this repository say this should not happen, and the code contradicts all three:

  1. ADR-0041 itself: "the column is left out of the write's response and read back by the next query." It is not left out of the response; it is left out of the statement and then serialised anyway.
  2. binding.go's own A computed column is declared on the model but wanted by one mount, so every query of that model pays for it — and a Needs() bind makes unrelated queries fail #92 rationale (rest/binding.go:39): a computed column a mount does not select is "absent from the JSON rather than present holding its zero value — which for a bool would have been indistinguishable from a real false". That is precisely the failure here, on the write path, for a column the mount did select.
  3. ADR-0041's context section argues the whole obligation machinery exists to stop "a declared-but-never-computed field from being a permanently-zero JSON key". On the write path the field is exactly that.

Why it matters to a real consumer

This is not hypothetical — it is the bug a consumer believes this feature deleted. studio-apps' newsfeed module was ported to sqlb specifically because its hand-written handlers had the EXISTS predicate in the list query and the get query and not in the update query, "so PATCH returned a post whose myAcknowledged was always false". The port's record celebrates: "Declared once, it cannot disagree with itself."

Under the generated resource, PATCH /newsfeed/{id} still answers myAcknowledged: false to a viewer who has acknowledged the post. The declaration did not delete the bug; it moved it from the consumer's handler into the runtime, where no consumer test looks for it — theirs pass because they assert on GET after PATCH.

The fix that fits the existing machinery

The response row already serialises only the columns in its projection, and a key absent from the projection is absent from the JSON — that machinery was built for #92. Excluding Needs-computed columns from the write-response projection (a writeSelectable beside selectable, or a filter at the two call sites in item.go) makes the write response honest: the key is absent, which a client reads as "not computed here", exactly the contract ADR-0041 wrote down.

The richer alternative — re-reading the row through the read path, which runs BeforeQuery and therefore has the bind — buys a fully-populated write response at the cost of a second statement. That is a separate decision; the absent key is correct either way, and is what the ADR already promised.

Found reviewing sqlb against the studio-apps port (SQLB-PORT-FEEDBACK.md finding 34).

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions