Uh oh!
There was an error while loading. Please reload this page.
fix(drivers): limit: 0 returns no records, on every driver and every read door (#6577) - #6793
Merged
Merged
Conversation
…w-function and explain doors (#6577) `findRows()` has always compiled `limit` on presence (`query.limit !== undefined`), which is what `limit: 0` — ruled in #6485 to mean "return no records" — depends on. Two other doors in the same file compiled it on truthiness, and 0 is falsy: findWithWindowFunctions() returns ROWS -> { limit: 0 } returned the whole table analyzeQuery() / explain() returns a PLAN -> explained a statement without the LIMIT Measured on 3172831 before the change: three rows seeded, find({limit:0}) -> 0 rows while findWithWindowFunctions({limit:0}) -> 3, and analyzeQuery emitted `select * from \`orders\`` where find() sent `... order by \`id\` asc limit ?`. `offset` moved with `limit` for internal consistency only: knex elides a zero offset on better-sqlite3, pg and mysql2 alike, so that half is measured to change no statement and no row set. Pinned as the no-op it is.
…emote bare-offset syntax error (#6577) Adds PAGINATION_ZERO_LIMIT_CASES — the shared case-set pinning #6485's ruling that `limit: 0` returns no records — with controls so "return nothing, always" cannot pass it. Answered by driver-sql, driver-sqlite-wasm and driver-turso on BOTH transports. driver-memory and driver-mongodb take honest DEBT rows: both are #5499-frozen and diverge for two different reasons (memory drops the slice on truthiness, measured 3-of-3; mongodb forwards 0 to a client that defines it as no-limit). The bare-offset control surfaced a separate live defect in turso's remote transport: it emitted LIMIT and OFFSET independently, so `{ offset: N }` with no limit assembled `... OFFSET ?` and SQLite answered `near "OFFSET": syntax error` — for every N, and only on remote (knex synthesises LIMIT -1 locally). Remote now builds the statement knex builds.
… options typed (#6577) - turso remote: the LIMIT -1 sentinel pins now read the SELECT off a recording client instead of calling the private `buildSelectSQL`, so they assert what the transport actually sent and survive `find()` changing how it builds it. Adds the third direction: neither limit nor offset given emits no LIMIT, so the sentinel is not unconditional. - drop the `as any` casts the new call sites had picked up, keeping check:query-options-erasure at its 263 ceiling rather than raising it.
… full card) Per the maintainer's #5499 freeze exception (ruling on #6577: 「A+ 批准，按这个范围执行」), scoped to the limit door only: - driver-memory: find() sliced on truthiness, so limit 0 returned the whole table (measured 3-of-3; with offset 1 it returned 2, the offset applying and the limit not). Presence now. Same shape fixed at the two memory-analytics sites; mingo honours $limit 0 as zero records (measured), so no short-circuit is needed there. - driver-mongodb: presence was already correct — the value was forwarded faithfully to a client that DEFINES limit 0 as 'no limit'. Answered before the client is consulted instead: [] from find, null from findOne. No round trip, and the upstream driver's reading of 0 can no longer decide the contract. - Conformance: PAGINATION_ZERO_LIMIT_CASES now answered by all five drivers, no DEBT rows. 33 covered, 2 DEBT (the untouched #6682 FILTER_TEXT rows). The #5499 freeze remains in force for everything else in both packages.
Drops the `as any` the memory/mongodb limit-0 blocks had picked up, holding check:query-options-erasure at its 263 ceiling instead of raising it to 270.
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 5 package(s): 118 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
August 8, 2026 18:19
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 8, 2026
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes#6577
limit: 0was ruled in #6485 to mean return no records. Three of the five shipped drivers did not honour it — in three genuinely different ways — and every one of them answered with more data than was requested, which on an ADR-0021 RLS read scope is over-reach rather than a loose filter (#3948).Reachable rather than theoretical: since #6578 the client puts
top=0on the wire, so the answer depended on which driver the deployment configured.Scope — the maintainer's #5499 freeze exception
Dispatched as the unfrozen
driver-sqlhalf, then re-scoped to the full card by the maintainer ruling on #6577 (comment5226535233, 2026-08-08T14:27Z), verbatim: 「A+ 批准，按这个范围执行」. The exception is the limit door only; the #5499 freeze remains in force for everything else indriver-memory/driver-mongodb— the #6682 case-folding programme is untouched, and both packages keep theirFILTER_TEXT_CASESDEBT rows exactly as #6706 left them.Premise re-verified against post-#6706
origin/main, measured not readHard precondition honoured: #6706 (issue #6518) touches the same file and was still open when this was dispatched. Polled
origin/mainuntil its merge commit3172831landed at 14:21:57Z; branched from that commit, never earlier. The two truthiness doors had drifted again (triage said:3922/:3962, #6706 moved them to:4197/:4237) — located by theif (query.limit) builder.limit(query.limit)pattern, never by line number.Probed on
3172831before any line moved, three rows seeded:premise_still_valid: true, on both halves.One standard, three mechanisms
find()sliced withif (query.limit)— truthiness,0is falsy, so the slice was dropped and a request for nothing got all 12 rows. The{ limit: 0, offset: 1 } -> 2reading is why every paging suite stayed green over it: the OFFSET applied and the LIMIT silently did not.memory-analytics.ts($limitstage, SQL string builder). Mingo honours{ $limit: 0 }as zero records (measured: 3 in, 0 out), so no short-circuit is needed there.buildFindOptionsalready tested presence, so0arrived exactly as written. The divergence is one layer lower: the MongoDB Node driver defineslimit: 0as no limit. Forwarding faithfully was the bug.[]fromfind,nullfromfindOne, which had the same hole. No round trip for a query whose answer is known, and no future change in the upstream driver's reading of0can move it. Deliberately=== 0, not<= 0: a negative limit is not a shape the contract defines, and folding it in would invent an answer the validation layer owns.findRows()(whatfind()uses) always compiled on presence;findWithWindowFunctions()(#4286 — returns rows) andanalyzeQuery()/explain()(returns a plan) compiled on truthiness. The plan door explainedselect * from "orders"wherefind()sent... order by "id" asc limit ?— an EXPLAIN for a different statement.OFFSETandLIMITwere emitted independently, but SQLite's grammar isLIMIT expr [OFFSET expr]. Sofind(obj, { offset: N })with nolimitproducednear "OFFSET": syntax error— for everyN, not a boundary value, and only on remote (local goes through knex).LIMIT -1no-limit sentinel, so both transports build the same statement. Measured: knex compiles.offset(3)tolimit ? offset ?bound[-1, 3].Result sets only ever get narrower. A caller who wants every row should omit
limitrather than pass0.offsetmoved withlimitin driver-sql — and the honest reasonBoth driver-sql doors also spelled
if (query.offset). That flip is measured to change nothing: knex elides a zero offset on better-sqlite3, Postgres and MySQL alike (compiled.offset(0)isselect * from "orders"on all three). It is made for internal consistency and pinned as the no-op it is, not sold as a fix. My first draft of the changeset claimed the statement changed; the measurement said otherwise and the claim was corrected.driver-memory'soffsetis deliberately left on truthiness:slice(0)is the identity slice, so presence and truthiness cannot be told apart there. No behaviour to fix, and the exception was scoped to the limit door.Reverse verification — direction predicted BEFORE each run
Each experiment reverts one arm and predicts a disjoint red set.
sql-driver.tsdoorslimit: 0cases + 2analyzeQueryLIMIT cases red; all 8 controls greenmemory-driver.tsslicemongodb-driver.tsguardSqliteError: near "OFFSET": syntax errorConformance — pinned across all five drivers, no DEBT
New shared case-set
PAGINATION_ZERO_LIMIT_CASESinpackages/spec/src/data/pagination-conformance.ts, classified incheck-driver-conformance.mjs.A separate marker from
PAGINATION_CASESrather than extra rows in it, deliberately: those cases are about a page being a partition, these are about the page size being read at all, and they fail independently. Keeping them apart is also what keeps the ledger honest — a driver answering one and not the other is a half-covered cell, and a shared marker would let it import its way to green.Three of the six cases are controls (
limit: 2→ 2, bareoffset: 0→ 12, no limit → 12). Without them a driver returning[]for everything would pass every zero case and fail its users completely — and for MongoDB specifically, the controls are what stop the new short-circuit from being an unconditional "return nothing".The 2 surviving DEBT rows are the untouched
FILTER_TEXT_CASESpair pointing at #6682 — frozen, and not this PR's business.Which cells executed, and which did not
declareUnprovisionedCell) rather than faking green — but CI'sTemporal Conformance (live PG + MySQL)job sets both URLs and runs this package against both servers. That job is green on12c8176, so the six newlimit: 0cases executed against live PG and MySQL, not only against embedded SQLite.mongodb-memory-servercould not fetch its binary (download blocked in this container), and CI's matrix does not provision one either; it is a declared skip by name, and the 19 live cases stayed skipped rather than reported green. The guard is still proved unconditionally: its server-free block runs on a driver that is never connected, against an unreachable URI by construction — so if the short-circuit were removed those cases would fail on a connection error, which is exactly the right alarm. For this particular contract that is the stronger instrument, since the claim is "no client is consulted".Gates — enumerated from
.github/workflows/lint.yml, run one by oneAll 54
check:*steps inlint.yml, run individually: all OK. Pluspnpm lintOK,turbo run typecheckOK (120/120),pnpm --filter @objectstack/spec exec tsc --noEmitOK,check:empty-changesetOK,check:adr-0087-registrationOK (no declared-breaking changeset).Two notes on gates that needed care:
check:query-options-erasurewent red twice, both times mine: the new test call sites had picked upas anyon the query argument (263 → 267, then → 270). Fixed by typing the call sites rather than by raising the ceiling — now back at 263, "no files added".check:i18n/check:i18n-coverage/check:app-nav-i18n/check:type-check-debtfail on an unbuilt tree and say so explicitly ("Nothing was measured"). Green afterturbo run build, exactly aslint.ymlorders them.pnpm test(whole repo, locally): one red,plugin-email's attachment-queue test, which timed out at 5s under 120-package parallel load (that run reported transform 74s / import 218s). Re-run alone: 302/302 pass. Unrelated to this diff — no driver, pagination or spec surface in it — and CI's shardedTest Core (1–3/3)is green, which is the authority over my single-box run.CI on
12c8176: all 25 checks green,mergeable_state: clean.Changeset
patchon@objectstack/driver-sql,@objectstack/driver-memory,@objectstack/driver-mongodb,@objectstack/driver-turso;minoron@objectstack/spec(additive export — no existing export moved).check:adr-0087-registrationconfirms no declared surface moves:limitstill exists and still takes a number; what changed is that the value is now honoured.Constraints honoured
FILTER_TEXT_CASESDEBT rows are byte-for-byte as fix(drivers): text-operator case folding is the contract's answer, not the dialect's (#6518) #6706 left them.content/docs/releases/: untouched. No pin bumps.as any/as never/@ts-ignorein the final diff; the erasure ratchet is at its ceiling, not above it.packages/specchange is one additive export plus its regeneratedapi-surface/data.json.Out-of-scope finding
findWithWindowFunctions()andanalyzeQuery()never callapplyTenantScope, while every other CRUD door insql-driver.tsdoes — and that method's own doc comment calls it "the single chokepoint for read-side tenant isolation in the SQL driver". The window-function door returns rows, so on a multi-tenant deployment it is a tenant-wall bypass, not merely a plan mismatch. Noticed while working these exact two methods; deliberately not fixed here — it is a different defect with a different blast radius and wants its own ruling, and folding a security fix into alimit: 0PR would bury it. Filed as [finding][drivers]SqlDriver.findWithWindowFunctionsandanalyzeQueryskipapplyTenantScope— a row-returning read door outside the driver's "single chokepoint" for tenant isolation #6792, unassigned, search-first-deduped, with its reachability question stated as the thing a grader must measure first.Generated by Claude Code