fix(#2): normative response-side tagged-value emission - #19
Merged
Conversation
SPEC.md section 6.1 named the section 5 encoding for row values without
obliging anyone to emit it -- the only server duty was inbound ("MUST
accept"). A server could return a stored INTEGER above 2^53 as a bare
JSON number and break no rule.
Adds normative emission rules keyed on the stored value rather than the
driver's runtime type, plus conformance cases V-1..V-4 that write via SQL
literal text so the value never passes through the request params (which
is why P-5 could not catch this). Documents the D1 / Durable Object
drivers as failing V-1: neither has a lossless 64-bit integer mode.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Section 6.1 named the section 5 tagged-value encoding for row values but never obliged a server to emit it -- section 5's only server duty ("Servers MUST accept the registered types") is inbound. A conforming server could therefore return a stored INTEGER above 2^53 as a bare JSON number and silently round it in every JavaScript client, and conformance case P-5 could not catch that because it only roundtrips a value the client itself sent. This branch makes emission normative, keyed on the stored value rather than on whatever runtime type the server's driver produced, adds conformance cases that write values as SQL literal text so they never pass through
params, and records both Cloudflare reference servers as failing the new integer case because neither D1 nor Durable Object SQL storage offers a lossless 64-bit integer mode.Acceptance criteria mapping
1. S6.1 puts a MUST on tagged-value emission, not just a cross-reference to S5
The
rowsbullet no longer stands alone as a cross-reference. It now reads "subject to the emission rules below", and a new "Response value encoding" subsection states the obligations directly: a server MUST emit an out-of-range integer as{"$type":"bigint",...}, MUST emit a binary value as{"$type":"blob",...}, and MUST NOT substitute a lossy representation (rounded number, truncated integer, re-formatted string) for the tagged form. The rules open by declaring they key on the stored value the statement produced, "not on whatever runtime type the server's database driver handed back", which is the sentence that makes a driver-rounded double a conformance failure rather than an encoding-layer detail. The JSON-safe range is defined inline as an explicit numeric interval so each MUST is decidable without consulting an implementation. Scope is deliberately held torows(including 6.2'sresultsentries);lastInsertIdis excluded because its declaredstring | number | nulltype would contradict a tagged-value MUST, and that truncation is separately tracked.Evidence: SPEC.md:118 (
rowsbullet) and the "#### Response value encoding" subsection immediately following it, containing three MUST/MUST NOT clauses plus a MAY and a SHOULD.2. Same normative treatment for BLOB values on the response side
BLOB emission is covered by its own MUST in the same block rather than being left to the section 5 reference, and the MUST NOT on lossy substitution applies to it as well. The conformance suite exercises it via a binary SQL literal so the blob is never handed to the server as a tagged param.
Evidence: SPEC.md, "A server MUST emit a binary value as
{"$type": "blob", "$value": "<base64>"}"; conformance/README.md case V-3,x'48656c6c6f'in,{"$type":"blob","$value":"SGVsbG8="}out.3. New conformance case: a value the client never sent roundtrips without loss
A "Response value encoding" section adds V-1 through V-4. V-1 inserts
9007199254740993as SQL literal text and SELECTs it back, asserting the tagged form and explicitly failing the rounded9007199254740992; V-2 covers the negative bound; V-3 covers a binary literal; V-4 pins that an in-range integer may come back as a plain JSON number so the SHOULD is not misread as a prohibition. Prose above the table states why these exist separately from P-1..P-5, and prose below states that V-1 is unpassable by an encoding layer that branches on the runtime type it was handed. The fixture gainsbig_value BIGINTandblob_value BLOB: this is load-bearing, because storing the V-1 literal in the existingbody TEXTcolumn would let SQLite's TEXT affinity coerce it to a string and the case would pass while proving nothing.Evidence: conformance/README.md, "### Response value encoding" table (V-1..V-4) and the amended
CREATE TABLE http_sql_conformance_notesfixture with its affinity note.4. Both reference servers addressed at the driver-config layer, verified against the new case
I traced this rather than assuming. Neither driver can be configured to return BigInt, so neither example can be made to pass V-1 by any code change in this repo: the rounding happens inside the driver before the Worker sees the row, and
encodeValue'stypeof value === "bigint"branch is reached only with an already-destroyed value. The Durable Objects storage documentation states that a largeint64"may be less precise than your original number" on retrieval andSqlStorageValueisArrayBuffer | string | number | nullwith no BigInt member; D1's documentation states it "supports 64-bit signed INTEGER values internally, however BigInts are not currently supported in the API yet"; the upstream request, cloudflare/workerd#4195 (opened 2025-05-19, still open, covering both D1 and DO SQLite mode), has no fix. Rather than close the issue with an implementation that still loses the value, both examples now carry the finding at three levels: a comment onencodeValuenaming the case it fails and quoting the API evidence, a bullet in each README's "does NOT do" list flagged as a known non-conformance rather than a design choice, and a note in the implementations directory so the self-asserted conformance table is honest. Each names the SQL-layer workaround (SELECT CAST(col AS TEXT)) and why it is not applied automatically -- doing so would require parsing the caller's SQL and inferring column types.Evidence: examples/cloudflare-durable-object/src/index.ts, the "KNOWN NON-CONFORMANCE (spec 6.1, conformance case V-1)" block above
encodeValue; examples/cloudflare-worker-to-d1/src/index.ts, the parallel block above itsencodeValue; implementations.md, the D1 and DO server rows.Not done
CAST(col AS TEXT)rewriting. It would require the servers to parse and rewrite caller SQL and infer column types, which is both a large behavior change for an example and against the spec's posture that the server executes the SQL it is given.lastInsertIdleft alone. It carries the same 64-bit truncation risk (last_row_idis a SQLite INTEGER) but its declaredstring | number | nulltype is the actual defect there, and fixing it is a separate normative change tracked apart from this issue.conformance/README.mdsays so under Status), so V-1..V-4 are specified against the contract the future runner will be built to, exactly as every existing case is.Closes #2