Repository navigation
fix(sqlite): fix a statement's size by its shape, not its query - #464
Merged
Merged
Conversation
Durable Object SQLite caps a statement at 100 bound parameters and 100 KB of text. The shared declaration promised contentByType and contentFieldByType at 64 keys, but each arm and entry bound its own parameters and text, so a query within the cap failed on DO; so did a plain filter.content with a few deep paths, which no cap bounds. Rather than lower the caps for one engine, every caller-sized part of a query is now data: contentFieldByType's entries bind as one list each, and all content conditions (filter.content, contentPresent, every contentByType arm) bind as one table of scoped rows whose paths a recursive query walks. The spec states the rule once, in adapters.md § Statement size, and the 32-segment path cap now rests on the content nesting cap rather than SQLite's join limit. Closes #462. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mf3p8BVtrQJLZx1JFCHchC
🦋 Changeset detectedLatest commit: 1c8b178 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The null op held only where the path reached no non-null value, which made it the exact complement of contentPresent. The spec has them overlap on a multi-valued path: [null, 'x'] satisfies both. It now holds where the walk reaches a null, or reaches nothing non-null, and the conformance suite pins the overlap for every adapter. Also from review: the content path cap reads the nesting constant rather than restating 32, and § Statement size says a statement never grows with a query's entries, naming the single-value binding and contentFieldByType's two lists. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mf3p8BVtrQJLZx1JFCHchC
Review found the SQL leaf drifting from matchesContent() again: a filter value of true matched a stored 1, because spread() rewrapped a boolean from json_each's 0/1 value. Booleans now keep their type through the walk, and an equals condition compares kind as well as value, as === does; the conformance suite pins it. Every caller-sized part of a query is now one primitive, rows(): a JSON array of tuples read as named columns. Lists, permission sets, contentFieldByType's two tables and the content conditions all use it. Each condition walks its path once and gathers what its op asks, which also halves the isNull cost. Ops are equals, present and isNull. The path cap is derived from the nesting cap instead of restating it: a top-level field sits at depth 0, so 33 segments reach the deepest value a write accepts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mf3p8BVtrQJLZx1JFCHchC
Once a per-Type key cost a SQLite statement no text or parameter of its own, the 64 local adapters declared had no cause left. The caps are now number | null: null states no limit, 0 still declines, and an absent or malformed cap still reads as 0. Local adapters declare null; a cap is for a server bounding its own cost. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mf3p8BVtrQJLZx1JFCHchC
admitsKeys(cap, keys) is now the only place a cap is read; query validation, collection narrowing and the conformance suite all call it rather than spelling null-means-no-limit three ways. readCap replaces capOrZero, whose name stopped describing it, and a discovery fixture pins a stated null cap on the wire. Also: the data-model sentence on object and array values now says what matchesContent() does, normalizeCapabilities() says which null readings need stating, and contentByType's type list says why it stays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mf3p8BVtrQJLZx1JFCHchC
Review found the same rule spelled more than one way again, both times because a primitive returned a fragment and left its policy to callers. rows() now registers a named, materialized table on the statement it builds, so every caller-sized table is parsed once per statement and binds first. The absent partition of a by-Type sort no longer re-parses its JSON per record, and statement() loses its parameter-ordering logic. perTypeCap(capabilities, shape) applies the reach rule wherever a cap is used; normalizeCapabilities() reads a cap as stated, collection narrowing and both capability asserts go through it, and assertSortCapability() now refuses a by-Type sort against a reach of 'none' as the spec says. null states no limit on a cap and a limit alike; only absence reads differently, by the least-capable rule. Also: holds() names the aliases it reads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mf3p8BVtrQJLZx1JFCHchC
…s enter What depends on filter.content — contentPresent and both per-Type caps — is false or 0 beside 'none'. That is now established where capabilities enter, as AGENTS.md asks: the conformance suite already holds a local adapter's declaration to it, and normalizeCapabilities() applies it to a discovery response, contentPresent included. Readers trust it, so perTypeCap() is gone and every cap is read directly through admitsKeys(). Also: collection narrowing skips an unused shape rather than calling it an unlimited cap; holds() names its isNull branch and derives depth from path instead of carrying both; valueKind() and kindOf() point at each other; holds() and normalizeCapabilities() docblocks fit the comment limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mf3p8BVtrQJLZx1JFCHchC
The per-Type caps were the only capabilities holding a count, and the statement limits that once justified one are gone. Nothing else a query carries is counted — content keys, presence paths and tag lists are all unbounded, and a server bounds their cost by its query deadline — so filter.contentByType and sort.contentFieldByType are now booleans, like filter.contentPresent. A collection facing false queries its handle's version alone and reports skipped, as it did at a cap of 0; partial narrowing goes, and with it admitsKeys(), the null cap and the 0/null absence asymmetry. Also: a Stack test ties the 33-segment path cap to write validation, and the conformance suite checks everything that depends on the content reach in one test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mf3p8BVtrQJLZx1JFCHchC
The invariant behind contentPresent and the per-Type flags is the POST /records/query body, not filtering: a per-Type sort needs that endpoint where sort.contentField, a query parameter, does not. The spec and the code that apply it now say so. elementsByRecord() reads its ids through rowsTable(), the shape rows() binds, and encodes them itself rather than taking a caller's JSON. The collection no longer orders a family by version, which only served the cap, and assertQueryCapabilities() reads contentByType with its siblings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mf3p8BVtrQJLZx1JFCHchC
Merged
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
Closes #462.
Durable Object SQLite limits a statement to 100 bound parameters and 100 KB of SQL text. The shared declaration promises
contentByTypeandcontentFieldByTypeat 64 keys, but each arm and entry bound its own parameters and text, so a query within the cap failed on DO. So did a plainfilter.contentwith a few deep paths, which no cap bounds.This PR doesn't give DO lower caps. Instead, every SQLite engine follows one rule: a statement's text and bound parameters never grow with how many entries a query's keys hold. One primitive implements it:
rows(), a JSON array of tuples. Each one is registered on the statement as a named, materialized table, so it binds first and its JSON is parsed once per statement. The batched element read inrecord-logic.tsuses the same table shape (rowsTable()).= ?, so an index keeps yielding rows in order. This is the rule's one carve-out, and the spec states it.contentFieldByTypeis two tables: Types ordered by a field, and Types ordered by a constant.filter.content,filter.contentPresentand everycontentByTypearm. Each row holds a path, an op (equals/present/isNull), a value with its JSON kind, and the Type it applies to (none forfilter.content). A record matches when every row that applies to it holds. One recursive walk per row,holds(c, r), matches a segment per step and collects everything the op needs.Per-Type shapes are flags. With the statement limits gone, nothing justified a count, and nothing else a query carries is counted:
filter.contentByTypeandsort.contentFieldByTypeare now booleans, likefilter.contentPresent. A server bounds the cost of an n-arm query by its query deadline, the same way it bounds n content keys.falsequeries its handle's version alone and reportsskipped, as it did at a cap of0. Partial narrowing (k of n versions) is gone.POST /records/querybody (contentPresentand both per-Type flags) isfalsebesidefilter.content: 'none', because a server reaching no content doesn't serve that endpoint. This depends on the endpoint, not on filtering:sort.contentFieldtravels as a query parameter, so it doesn't depend on it. It is established where capabilities enter: the conformance suite checks local adapters' declarations, andnormalizeCapabilities()applies it to discovery responses, now includingcontentPresent.The path cap is
MAX_VALIDATION_DEPTH + 1(33). A top-level field sits at depth 0, so 33 segments reach the deepest value a write accepts.Matching follows
matchesContent()in core, which is the reference reading:===does. Booleans now keep their type through the walk. Onmain, a filter for1matched a storedtrue.isNullandpresent.Spec
docs/spec/adapters.md§ Statement size holds the rule. § Querying within a reach points to it.falsebelow'none', enforced where capabilities enter.docs/spec/data-model.md:falseflag gets the handle-only query.docs/spec/wire-format.md: the discovery example usestrue, a server declaringfalserefuses with400, and § Bounding query cost covers per-Type arms.Verification
pnpm run format:check,check:refs,lint,test,buildandtypecheckall pass.contentPresentand anullfilter.true,1,'1').falsebelow'none', checked in one test.contentByTypeandcontentFieldByTypequeries with 200 keys, and a content filter far past both DO limits. All three failed onmain.Stacktest writes content nested to the deepest accepted level and matches it with a 33-segment path. Content one level deeper is refused at write, and a 34-segment path is refused at query.trueonly when statedtrue, and every query-body flag readsfalsebeside'none'.EXPLAIN QUERY PLANby hand:idx_content_index_sort,idx_records_type_idandidx_permissions_elementstill drive their queries.content_indexby(record_id, field).Notes for reviewers
equalsandpresent(132 ms → ~300 ms on 50k records, node SQLite). Paged queries are unaffected. Walking once per condition madeisNull1.4–2× faster than walking twice. This was accepted in exchange for the single shape; the alternatives were a per-engine segment budget, or a second, faster matching path to keep in sync with the first.🤖 Generated with Claude Code
https://claude.ai/code/session_01Mf3p8BVtrQJLZx1JFCHchC