From f675f691d50e6551940f4f3e2c2fbd12ebd85d43 Mon Sep 17 00:00:00 2001 From: Sean Silvius Date: Wed, 29 Jul 2026 16:56:13 -0700 Subject: [PATCH 1/3] spec: define non-atomic batch failure behavior (#1) SPEC.md S6.2 covered only the atomic failure case. The rule a client must actually code against lived at conformance/README.md:51 -- a document S1 gives no normative authority -- and that document invented a permission the spec never granted: 200 with partial results. Three clauses, adjacent because the adjacency is the point: 1. Forbid partial success. A results array MUST match the batch length; a failure returns the error envelope for atomic AND non-atomic. 2. error.statementIndex is REQUIRED for non-atomic batch failures, RECOMMENDED for atomic. Scoped deliberately: both reference servers attach it only on the non-atomic path (worker/DO runBatch), and D1's batch failure surfaces no per-statement index -- an unscoped MUST would make both non-conforming on day one. Correction from smugglr. 3. State that preceding statements REMAIN COMMITTED, with the client MUST NOT in the same block. This sentence existed nowhere. B-4's "fail closed" sitting under B-3's "no rows persisted" reads as nothing-applied, and that false belief corrupts a migration ledger on replay. Verified against both reference servers before writing: on a non-atomic failure each throws and discards the accumulated results, so clause 1 costs them nothing. The conformance doc granted a permission no implementation exercises. Also in scope, beyond the issue's original "spec text only": - conformance/README.md B-3/B-4 rewritten to assert what persists, plus a new B-5 covering that statements AFTER the failure did not execute. Fixing SPEC.md alone would leave the contradicting permission live in the document clients actually read. - examples/reference-client.ts carries appliedCount. The new client MUST is unsatisfiable by a client that discards how far the batch got. undefined means unknown, never zero -- a caller reading zero replays the whole batch. Not touched: S10.1's "honor atomic when not rejected" is issue #3. Co-Authored-By: Claude Opus 5 (1M context) --- SPEC.md | 23 ++++++++++++++++++----- conformance/README.md | 5 +++-- examples/reference-client.ts | 16 +++++++++++++--- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/SPEC.md b/SPEC.md index e115dab..d7fb2f7 100644 --- a/SPEC.md +++ b/SPEC.md @@ -136,7 +136,18 @@ HTTP status: `200`. - `results` (REQUIRED, array of single-statement result objects) — in the same order as the request `batch`. Each element has the shape of section 6.1. -If an atomic batch fails partway through, the response is the error envelope in section 7, not a partial `results` array. +A `results` array MUST have the same length as the request `batch`. A server MUST NOT return status `200` with a `results` array covering only some of the submitted statements. + +#### 6.2.1 Batch failure + +If a statement in a batch fails, the response is the error envelope in section 7, not a partial `results` array. This holds for both atomic and non-atomic batches. + +The two cases differ in what persists, and the difference is normative: + +- **Atomic batch** (`atomic: true`) — the transaction rolls back. No statement in the batch has any effect. +- **Non-atomic batch** (`atomic` absent or `false`) — statements preceding the failing statement have been executed and their effects persist. The failing statement and all statements after it have not been executed. Clients MUST NOT treat a non-atomic batch error as no-statements-applied. + +For a non-atomic batch failure the server MUST include `error.statementIndex` (section 7), because it is the only means by which a client can determine how far the batch got. For an atomic batch failure the server SHOULD include it where the underlying engine surfaces it; recovery does not require it, but diagnosis of a long batch does. ## 7. Error responses @@ -154,7 +165,7 @@ HTTP status: `4xx` or `5xx`. - `error.code` (REQUIRED, string) — one of the registered codes below, or a vendor-namespaced code (`vendor:`). - `error.message` (REQUIRED, string) — human-readable explanation. Servers SHOULD avoid leaking sensitive details. -- `error.statementIndex` (OPTIONAL, integer) — for batch requests, the zero-based index of the statement that failed. Omitted for single-statement requests. +- `error.statementIndex` (REQUIRED for non-atomic batch failures, otherwise OPTIONAL, integer) — the zero-based index of the statement that failed. For a non-atomic batch failure it is the client's only means of determining which statements persisted (section 6.2.1), so it MUST be present. For an atomic batch failure it is RECOMMENDED. MUST be omitted for single-statement requests. Registered error codes in v0.1: @@ -204,8 +215,9 @@ A v0.1 conforming server MUST: 3. Return the success envelopes defined in section 6 for successful execution. 4. Return the error envelope defined in section 7 for any failure, using the HTTP status codes in the table. 5. Honor `atomic: true` on batch requests when not rejected. -6. Accept the registered parameter types in section 5 (`blob`, `bigint`). -7. Emit the `X-Http-Sql-Version` response header. +6. On a batch statement failure, return the error envelope rather than a partial `results` array, and include `error.statementIndex` when the batch was non-atomic (section 6.2.1). +7. Accept the registered parameter types in section 5 (`blob`, `bigint`). +8. Emit the `X-Http-Sql-Version` response header. A v0.1 conforming server MAY: @@ -220,7 +232,8 @@ A v0.1 conforming client MUST: 2. Send exactly one of `sql` or `batch` in the request body. 3. Use the standard parameter encoding from section 5. 4. Handle the success and error envelopes from sections 6 and 7. -5. Not require any vendor-specific request or response fields beyond those defined here. +5. On a non-atomic batch error, treat the statements preceding `error.statementIndex` as applied (section 6.2.1). A client MUST NOT assume no statements were applied. +6. Not require any vendor-specific request or response fields beyond those defined here. A v0.1 conforming client SHOULD: diff --git a/conformance/README.md b/conformance/README.md index d4f1811..58f3e27 100644 --- a/conformance/README.md +++ b/conformance/README.md @@ -47,8 +47,9 @@ Conformance is self-asserted. The community can call out failures via issues. |-------|------------------------------------------------------------------|---------------------------------------------| | B-1 | Two INSERTs, no `atomic` | 200, `results` array length 2 | | B-2 | Two INSERTs with `atomic: true` | 200, `results` array length 2 | -| B-3 | Atomic batch where the second statement fails | 400, error envelope, no rows persisted | -| B-4 | Non-atomic batch where the second statement fails | 400, error envelope (servers MAY also return 200 with partial results -- the spec leaves this implementation-defined; recommended behavior is to fail closed) | +| B-3 | Atomic batch where the second statement fails | 400, error envelope. NO statement persisted -- the first INSERT is absent. | +| B-4 | Non-atomic batch where the second statement fails | 400, error envelope, `error.statementIndex` = 1. The FIRST statement PERSISTS -- its INSERT is present. | +| B-5 | Non-atomic three-statement batch where the second fails | 400, error envelope, `error.statementIndex` = 1. First statement persists, THIRD did not execute. | ### Parameter types diff --git a/examples/reference-client.ts b/examples/reference-client.ts index 4a71966..daa67e2 100644 --- a/examples/reference-client.ts +++ b/examples/reference-client.ts @@ -27,10 +27,11 @@ export class HttpSqlClient { } async batch(statements: Statement[], atomic = false): Promise { - return this.send({ batch: statements, atomic }) as Promise; + return this.send({ batch: statements, atomic }, atomic) as Promise; } - private async send(body: unknown): Promise { + // `atomic` is undefined for single-statement requests. + private async send(body: unknown, atomic?: boolean): Promise { const res = await fetch(this.endpoint, { method: "POST", headers: { @@ -48,7 +49,16 @@ export class HttpSqlClient { code: "internal_error", message: `HTTP ${res.status}`, }; - throw Object.assign(new Error(err.message), { code: err.code, statementIndex: err.statementIndex }); + // Section 6.2.1: a non-atomic batch error does NOT mean nothing applied. + // Statements before statementIndex persisted. `undefined` means unknown -- + // never zero, because a caller that reads zero will replay the whole batch. + const appliedCount = atomic === undefined ? undefined : atomic ? 0 : err.statementIndex; + + throw Object.assign(new Error(err.message), { + code: err.code, + statementIndex: err.statementIndex, + appliedCount, + }); } return json; From e86d06cb3dbb32a985532582231fa977b62bb6a1 Mon Sep 17 00:00:00 2001 From: Sean Silvius Date: Wed, 29 Jul 2026 16:58:11 -0700 Subject: [PATCH 2/3] spec: drop the atomic-path statementIndex RECOMMENDED (#1) Scope drift, mine. Issue #1 considered a SHOULD-emit on the atomic path and explicitly did NOT file it: "Zero consumers need it, D1 cannot satisfy it, and an unsatisfiable SHOULD is surface for its own sake." I wrote it in from an earlier reflection that predated that reasoning, without re-reading the issue I was implementing. It was also the wrong shape twice over. "SHOULD include it where the underlying engine surfaces it" is a normative keyword qualified by a condition the spec does not define -- which is issue #3's exact defect ("honor atomic when not rejected"), reintroduced inside the fix for #1. S6.2.1 now states the absence positively rather than leaving a gap: nothing persisted on an atomic failure, so there is nothing to locate. Co-Authored-By: Claude Opus 5 (1M context) --- SPEC.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SPEC.md b/SPEC.md index d7fb2f7..31e9dbc 100644 --- a/SPEC.md +++ b/SPEC.md @@ -147,7 +147,7 @@ The two cases differ in what persists, and the difference is normative: - **Atomic batch** (`atomic: true`) — the transaction rolls back. No statement in the batch has any effect. - **Non-atomic batch** (`atomic` absent or `false`) — statements preceding the failing statement have been executed and their effects persist. The failing statement and all statements after it have not been executed. Clients MUST NOT treat a non-atomic batch error as no-statements-applied. -For a non-atomic batch failure the server MUST include `error.statementIndex` (section 7), because it is the only means by which a client can determine how far the batch got. For an atomic batch failure the server SHOULD include it where the underlying engine surfaces it; recovery does not require it, but diagnosis of a long batch does. +For a non-atomic batch failure the server MUST include `error.statementIndex` (section 7), because it is the only means by which a client can determine how far the batch got. No such obligation applies to an atomic batch failure: nothing persisted, so there is nothing for the client to locate. ## 7. Error responses @@ -165,7 +165,7 @@ HTTP status: `4xx` or `5xx`. - `error.code` (REQUIRED, string) — one of the registered codes below, or a vendor-namespaced code (`vendor:`). - `error.message` (REQUIRED, string) — human-readable explanation. Servers SHOULD avoid leaking sensitive details. -- `error.statementIndex` (REQUIRED for non-atomic batch failures, otherwise OPTIONAL, integer) — the zero-based index of the statement that failed. For a non-atomic batch failure it is the client's only means of determining which statements persisted (section 6.2.1), so it MUST be present. For an atomic batch failure it is RECOMMENDED. MUST be omitted for single-statement requests. +- `error.statementIndex` (REQUIRED for non-atomic batch failures, otherwise OPTIONAL, integer) — the zero-based index of the statement that failed. For a non-atomic batch failure it is the client's only means of determining which statements persisted (section 6.2.1), so it MUST be present. MUST be omitted for single-statement requests. Registered error codes in v0.1: From e79cb52a1b9068d09f53d8bf0a877331bc9dcab1 Mon Sep 17 00:00:00 2001 From: Sean Silvius Date: Wed, 29 Jul 2026 17:09:54 -0700 Subject: [PATCH 3/3] spec: state the execution-order premise clause 3 rests on (#1) Third instance of the same defect on this branch, and the one that would have shipped. Clause 3 asserts that statements before the failure persisted and statements after it did not execute. Nothing in SPEC.md made that true. S6.2's results bullet says "in the same order as the request batch" -- that is RESPONSE ordering. S4.2 describes batch as an array and says nothing about how statements are run. So a server that fanned a non-atomic batch out concurrently conformed to every word of the spec and falsified clause 3: a later statement can commit while an earlier one fails, and then statementIndex does not partition the batch at all. The sentence a migration applier acts on was resting on a mechanism the spec never defined -- issue #3's shape, for the third time here. Free, verified the way clause 1 was: both non-atomic runBatch paths are sequential loops that stop at the first failure -- the worker awaits runOne inside `for (let i...)` (index.ts:96-103), the DO calls runOne synchronously in the same shape (index.ts:122-131). Neither pays anything for the MUST. Atomic batches are deliberately exempt: the transaction makes execution order unobservable. Also, reference-client appliedCount: undefined was covering two different situations -- a single-statement request AND a non-atomic batch whose server omitted the REQUIRED index. Only the second is unknown; a failed single statement applied nothing, which is 0. Now `atomic === false ? err.statementIndex : 0`, so undefined means exactly one thing and a caller can branch on it. Co-Authored-By: Claude Opus 5 (1M context) --- SPEC.md | 11 +++++++---- examples/reference-client.ts | 11 +++++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/SPEC.md b/SPEC.md index 31e9dbc..d646f40 100644 --- a/SPEC.md +++ b/SPEC.md @@ -142,7 +142,9 @@ A `results` array MUST have the same length as the request `batch`. A server MUS If a statement in a batch fails, the response is the error envelope in section 7, not a partial `results` array. This holds for both atomic and non-atomic batches. -The two cases differ in what persists, and the difference is normative: +Servers MUST execute the statements of a non-atomic batch one at a time, in `batch` array order, and MUST stop at the first statement that fails. Without this, `error.statementIndex` does not partition the batch: a server that executed statements concurrently could commit a later statement while an earlier one failed, and the client could conclude nothing about what persisted. Atomic batches carry no ordering obligation, because the transaction makes execution order unobservable. + +Given that, the two cases differ in what persists, and the difference is normative: - **Atomic batch** (`atomic: true`) — the transaction rolls back. No statement in the batch has any effect. - **Non-atomic batch** (`atomic` absent or `false`) — statements preceding the failing statement have been executed and their effects persist. The failing statement and all statements after it have not been executed. Clients MUST NOT treat a non-atomic batch error as no-statements-applied. @@ -215,9 +217,10 @@ A v0.1 conforming server MUST: 3. Return the success envelopes defined in section 6 for successful execution. 4. Return the error envelope defined in section 7 for any failure, using the HTTP status codes in the table. 5. Honor `atomic: true` on batch requests when not rejected. -6. On a batch statement failure, return the error envelope rather than a partial `results` array, and include `error.statementIndex` when the batch was non-atomic (section 6.2.1). -7. Accept the registered parameter types in section 5 (`blob`, `bigint`). -8. Emit the `X-Http-Sql-Version` response header. +6. Execute a non-atomic batch sequentially in array order, stopping at the first failure (section 6.2.1). +7. On a batch statement failure, return the error envelope rather than a partial `results` array, and include `error.statementIndex` when the batch was non-atomic (section 6.2.1). +8. Accept the registered parameter types in section 5 (`blob`, `bigint`). +9. Emit the `X-Http-Sql-Version` response header. A v0.1 conforming server MAY: diff --git a/examples/reference-client.ts b/examples/reference-client.ts index daa67e2..8430607 100644 --- a/examples/reference-client.ts +++ b/examples/reference-client.ts @@ -49,10 +49,13 @@ export class HttpSqlClient { code: "internal_error", message: `HTTP ${res.status}`, }; - // Section 6.2.1: a non-atomic batch error does NOT mean nothing applied. - // Statements before statementIndex persisted. `undefined` means unknown -- - // never zero, because a caller that reads zero will replay the whole batch. - const appliedCount = atomic === undefined ? undefined : atomic ? 0 : err.statementIndex; + // Section 6.2.1: a non-atomic batch error does NOT mean nothing applied -- + // statements before statementIndex persisted. Atomic batches rolled back, + // and a failed single statement applied nothing, so both are 0. + // `undefined` means exactly one thing: a non-atomic batch whose server + // omitted the REQUIRED statementIndex, so how far it got is UNKNOWN. + // Never treat that as zero -- a caller reading zero replays applied work. + const appliedCount = atomic === false ? err.statementIndex : 0; throw Object.assign(new Error(err.message), { code: err.code,