Skip to content

Commit 8b50cb3

Browse files
os-zhuangclaude
andauthored
fix(spec,plugins): a paged read with no orderBy is a partition too — the shape every list view actually sends (#4363) (#4378)
* fix(data): paging a sorted read is a partition of the result set, not five queries that share a WHERE clause (objectui#3106) `ORDER BY status LIMIT 50 OFFSET 50` names a sort key that does not identify a row, and no backend promises that rows with equal keys keep the same relative arrangement between two queries. MongoDB documents this outright: `sort` + `skip`/`limit` on a non-unique key may return the same document more than once. Page 2 then repeats a row page 1 already showed and skips one nobody ever sees — with every page full, every row real, and the two halves of the symptom several screens apart. SqlDriver and MongoDBDriver append a unique tie-breaker to any non-empty `orderBy`, in the last requested key's direction: determinism holds either way, but a same-direction suffix is the one an index can still walk in a single pass. SqlDriver applies it only to objects it created itself (`initObjects` records those in `managedObjectFields`). A federated table (ADR-0015) may carry no `id` column, and guessing there would be worse than doing nothing — the resulting unknown-column error is answered by #3821's recovery ladder retrying with NO ORDER BY at all, trading a reshuffle among ties for the loss of the caller's whole sort. driver-memory needed no change: `Array#sort` is stable and the backing table's order does not move between reads. It gets a suite anyway, because that guarantee is implicit and is exactly what a refactor that looks like a speed-up (a hand-rolled sort, or sorting the array in place) would silently remove. The obligation is normative on `IDataDriver.find` and the cases are shared (`PAGINATION_CASES` in `@objectstack/spec/data`), so a future driver is held to it by a gate rather than by remembering. A paged read with NO `orderBy` is deliberately out of scope and filed as #4363. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(spec,plugins): a paged read with no orderBy is a partition too (#4363) objectui#3106's server half closed the SORTED paged read: a non-empty `orderBy` carries a unique tie-breaker, so `ORDER BY status LIMIT 50 OFFSET 50` can no longer serve one row twice while never serving another. It stopped there deliberately. This closes the half it left, which is the more common one. A list view whose metadata configures no `sort`, on which nobody has clicked a column header, sends no `$orderby` at all. `SqlDriver` and `MongoDBDriver` then emitted a bare LIMIT/OFFSET, and neither backend promises anything about the order that slices: SQL leaves the row order of an unordered read to the plan, and MongoDB's natural order moves when a document does. Every row ties with every other on an empty sort key, so it is the same defect at full strength rather than a different one. Both drivers now order a paged read by their unique key column when the caller supplied no sort keys -- the same `id` the tie-breaker was already appending, standing alone. `driver-memory` again needed no change: it slices its backing array, and two reads with no write between them see the identical sequence. The contract asks for a partition, not id order. Unpaged reads are untouched, deliberately: the rule keys off `limit`/`offset`, not off `orderBy` being absent. `limit` alone does count -- page one of a walk is routinely `limit=50` with no offset, and ordering only the later pages would leave the defect fully intact. `findOne` is outside all of this, and the contract now says so. Engines reach a driver with `limit: 1`, shaped exactly like page one of a walk, but it promises A matching record rather than a position in a sequence. Reading it as a page would put `ORDER BY id LIMIT 1` on the hottest read in the system, the classic shape for a planner to abandon the predicate's own index: measured on Postgres 16 over 2M rows, `WHERE owner_id = ? LIMIT 1` went 0.08ms -> 7.8ms and swapped the owner_id index for the primary key. `MongoDBDriver.findOne` has never sorted, so this also puts the two drivers back in step. Federated tables (ADR-0015) keep the existing carve-out -- no ordering column can be guessed there -- but now say so once per object, because the contract states determinism as a MUST and a MUST that quietly does not hold is the failure the rule was written against. The obligation is normative on `IDataDriver.find` and the cases are shared (`PAGINATION_UNORDERED_CASES` beside `PAGINATION_CASES`), so a future driver is held to both halves by a gate rather than by memory. Also repairs two holes in the tie-breaker suites this builds on: `InspectableSqlDriver` reimplemented the ORDER BY instead of observing the one `find()` emits (green on the day `find()` stops calling it), and MongoDB's pure sort-spec assertions sat behind a `skipIf` on a server they do not need -- so the only check that catches a deleted feature went silent, and reported green, wherever the mongod binary cannot be fetched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016niiseyRn43L9M4JdAwuqD --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 7924a27 commit 8b50cb3

13 files changed

Lines changed: 690 additions & 141 deletions

File tree

.changeset/paged-read-determinism.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ The obligation is now normative on `IDataDriver.find`, with shared cases in
3939
`@objectstack/spec/data` (`PAGINATION_CASES`) that all three drivers run — so a
4040
future driver is held to it by a gate rather than by remembering.
4141

42-
Deliberately not covered: a paged read with **no** `orderBy`. That is
43-
non-deterministic on every backend by definition and imposing an order on
44-
callers who asked for none changes plan selection far more broadly; filed as
45-
#4363.
42+
Not covered by this change: a paged read with **no** `orderBy`. Same defect,
43+
wider blast radius, so it was carved out to #4363 rather than folded in — and
44+
closed there, in the same release. The contract, the shared cases and both
45+
drivers now cover a paged read whatever its `orderBy`, including none at all.
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
---
2+
"@objectstack/spec": patch
3+
"@objectstack/driver-sql": patch
4+
"@objectstack/driver-mongodb": patch
5+
---
6+
7+
fix(data): a paged read with no `orderBy` is a partition too — the shape every list view actually sends (#4363)
8+
9+
objectui#3106's server half closed the **sorted** paged read: a non-empty
10+
`orderBy` now carries a unique tie-breaker, so `ORDER BY status LIMIT 50 OFFSET
11+
50` can no longer serve one row twice while never serving another. It stopped
12+
there deliberately. This closes the half it left, which is the more common one.
13+
14+
A list view whose metadata configures no `sort`, on which nobody has clicked a
15+
column header, sends no `$orderby` at all. `SqlDriver` and `MongoDBDriver` then
16+
emitted a bare `LIMIT`/`OFFSET` — and neither backend promises anything about
17+
the order that slices:
18+
19+
- **SQL** leaves the row order of an unordered read to the plan. Small tables
20+
hand back insertion order in practice, which is exactly why this survives
21+
testing; a parallel scan, an index scan, or a `VACUUM` need not.
22+
- **MongoDB** returns natural order, which describes where a document currently
23+
sits in its extent — and moves when the document does.
24+
25+
Every row ties with every other on an empty sort key, so this is the same defect
26+
at full strength rather than a different one: page 2 repeats a row page 1 showed
27+
and drops one nobody sees, with every page full and every row real.
28+
29+
Both drivers now order a paged read by their unique key column when the caller
30+
supplied no sort keys — the same `id` the tie-breaker was already appending, now
31+
standing alone. `driver-memory` again needed no change: it slices its backing
32+
array, and two reads with no write between them see the identical sequence. The
33+
contract asks for a partition, not for id order.
34+
35+
**Unpaged reads are untouched, deliberately.** The rule keys off `limit`/
36+
`offset`, not off `orderBy` being absent. A read with neither hands back the
37+
whole matching set, so no caller can be shown a partial view of it, and sorting
38+
every read in the system would change plan selection to buy nothing. `limit`
39+
alone does count as paged: page one of a walk is routinely `limit=50` with no
40+
offset, and ordering only the later pages would leave the defect fully intact.
41+
42+
`SqlDriver` keeps the existing restriction to objects it created itself
43+
(`initObjects` records them). It matters more here than for the sorted case: on
44+
a federated table (ADR-0015) there is no requested sort for #3821's ladder to
45+
fall back to, so a wrong guess about `id` would turn a reshuffle into a failed
46+
read. Those tables now get a warning — once per object, behavior unchanged —
47+
because the contract states determinism as a MUST, and a MUST that quietly does
48+
not hold is the same invisible failure the rule was written against.
49+
50+
`findOne` is deliberately outside all of this, and the contract now says so.
51+
Engines reach a driver with `limit: 1`, which is shaped exactly like page one of
52+
a walk, but it promises *a* matching record rather than a position in a
53+
sequence — nothing for a second call to be inconsistent with. Reading it as a
54+
page would put `ORDER BY id LIMIT 1` on the hottest read in the system, which is
55+
the classic shape for a planner to abandon the predicate's own index: measured
56+
on Postgres 16 over 2M rows, `WHERE owner_id = ? LIMIT 1` went 0.08 ms → 7.8 ms
57+
and swapped the `owner_id` index for the primary key. `MongoDBDriver.findOne`
58+
has never sorted, so this also puts the two drivers back in step.
59+
60+
The obligation is normative on `IDataDriver.find` and the cases are shared —
61+
`PAGINATION_UNORDERED_CASES` alongside `PAGINATION_CASES` in
62+
`@objectstack/spec/data` — so a future driver is held to both halves by a gate
63+
rather than by remembering.

content/docs/data-modeling/queries.mdx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,21 @@ reaching `engine.find()` directly are unaffected.
239239
}
240240
```
241241

242+
<Callout type="info">
243+
**Walking the pages visits every row exactly once**, whatever you sort by — or
244+
whether you sort at all. The SQL and MongoDB drivers get there by ordering on a
245+
unique column of their own, on top of whatever `orderBy` you gave, because a
246+
sort key like `status` does not identify a row and no backend promises equal
247+
keys keep the same arrangement between two queries. Without that, page 2 repeats
248+
a row page 1 already showed and skips one nobody ever sees — every page full,
249+
every row real, the two halves of the symptom several screens apart
250+
(objectui#3106, #4363).
251+
252+
The guarantee attaches to `limit`/`offset`, so it costs nothing on the reads
253+
that do not paginate: a query with neither is returned in whatever order the
254+
backend chooses, exactly as before.
255+
</Callout>
256+
242257
### Keyset Pagination — a `where` predicate on the sort key
243258

244259
`query.cursor` was **removed in `@objectstack/spec` 18** (#4286): nothing on the server

content/docs/protocol/objectql/query-syntax.mdx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,15 @@ const page2 = await engine.find('customer', {
859859

860860
**Drawback:** Slow for large offsets (database still scans all skipped rows).
861861

862+
**Determinism:** the walk above visits every `customer` exactly once even though
863+
it names no `orderBy`. A driver orders any paged read by a unique column of its
864+
own — appended to your sort keys, or standing alone when you gave none — because
865+
an unordered `LIMIT`/`OFFSET` slices an arrangement nothing holds steady: SQL
866+
leaves row order to the plan, and MongoDB's natural order moves when a document
867+
does. Page 2 would otherwise repeat a row from page 1 and drop another, with
868+
every page full and every row real (objectui#3106, #4363). A query with no
869+
`limit`/`offset` is untouched — nothing is being sliced, so no order is imposed.
870+
862871
### Keyset Pagination
863872

864873
<Callout type="warn">

packages/plugins/driver-memory/src/memory-pagination-conformance.test.ts

Lines changed: 58 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,41 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

33
/**
4-
* Deterministic paged reads for the in-memory driver (objectui#3106) — the
5-
* contract on `IDataDriver.find`, checked against the shared cases in
6-
* `@objectstack/spec/data`.
4+
* Deterministic paged reads for the in-memory driver (objectui#3106,
5+
* objectstack#4363) — the contract on `IDataDriver.find`, checked against the
6+
* shared cases in `@objectstack/spec/data`.
77
*
8-
* This driver needed **no change** to satisfy it, and that is worth a suite
9-
* rather than a shrug. It sorts with `Array#sort`, which ES2019 onward
10-
* guarantees is stable, over a table array whose order does not move between
11-
* two reads — so equal keys keep the same relative arrangement on page 2 that
12-
* they had on page 1, which is precisely what the contract asks for.
8+
* This driver needed **no change** to satisfy either half, and that is worth a
9+
* suite rather than a shrug.
10+
*
11+
* - **Sorted pages.** It sorts with `Array#sort`, which ES2019 onward
12+
* guarantees is stable, over a table array whose order does not move between
13+
* two reads — so equal keys keep the same relative arrangement on page 2 that
14+
* they had on page 1.
15+
* - **Unsorted pages.** With no `orderBy` it slices that same array directly,
16+
* and the array is the storage: two reads with no write between them see the
17+
* identical sequence. There is no plan to change its mind and no natural
18+
* order to move, so the walk partitions the set without a sort being imposed.
19+
* The contract asks for determinism, not for id order, and this driver
20+
* supplies it from a different direction than SQL and MongoDB do.
1321
*
1422
* The guarantee is therefore load-bearing but implicit: it rests on `sort`
15-
* being stable and on `applySort` copying rather than reordering the table in
16-
* place. Both are easy to lose in a refactor that looks like a speed-up — a
17-
* hand-rolled quicksort, or sorting the backing array directly — and neither
18-
* loss would fail any other test in this package. That is what this file is
19-
* for: it holds the property against the day the implementation changes, which
20-
* is the only day it could break.
23+
* being stable, on `applySort` copying rather than reordering the table in
24+
* place, and on `find` slicing a copy rather than the live table. All are easy
25+
* to lose in a refactor that looks like a speed-up — a hand-rolled quicksort,
26+
* sorting the backing array directly, a "reuse the array" allocation saving —
27+
* and no other test in this package would fail. That is what this file is for:
28+
* it holds the property against the day the implementation changes, which is
29+
* the only day it could break.
2130
*/
2231

2332
import { describe, it, expect, beforeEach } from 'vitest';
24-
import { PAGINATION_ALL_IDS, PAGINATION_CASES, PAGINATION_ROWS } from '@objectstack/spec/data';
33+
import {
34+
PAGINATION_ALL_IDS,
35+
PAGINATION_CASES,
36+
PAGINATION_ROWS,
37+
PAGINATION_UNORDERED_CASES,
38+
} from '@objectstack/spec/data';
2539
import { InMemoryDriver } from './memory-driver.js';
2640

2741
describe('InMemoryDriver — paged reads are a partition of the result set (objectui#3106)', () => {
@@ -67,4 +81,33 @@ describe('InMemoryDriver — paged reads are a partition of the result set (obje
6781
expect(paged.map((r) => r.id)).toEqual(whole.map((r: any) => r.id));
6882
});
6983
}
84+
85+
for (const testCase of PAGINATION_UNORDERED_CASES) {
86+
it(`visits every row exactly once with NO orderBy at all — ${testCase.name}`, async () => {
87+
const seen: string[] = [];
88+
for (let offset = 0; offset < PAGINATION_ROWS.length; offset += testCase.pageSize) {
89+
const page = await driver.find('ticket', { limit: testCase.pageSize, offset } as any);
90+
seen.push(...page.map((r: any) => String(r.id)));
91+
}
92+
93+
expect(seen).toHaveLength(PAGINATION_ALL_IDS.length);
94+
expect(new Set(seen).size).toBe(PAGINATION_ALL_IDS.length);
95+
expect([...seen].sort()).toEqual([...PAGINATION_ALL_IDS].sort());
96+
});
97+
98+
it(`page boundaries are invisible with NO orderBy — ${testCase.name}`, async () => {
99+
const paged: any[] = [];
100+
for (let offset = 0; offset < PAGINATION_ROWS.length; offset += testCase.pageSize) {
101+
const page = await driver.find('ticket', { limit: testCase.pageSize, offset } as any);
102+
paged.push(...page);
103+
}
104+
105+
// Concatenating the pages reproduces the unpaged read exactly. Note this
106+
// is insertion order, NOT id order: the contract asks for a partition,
107+
// and this driver's storage already provides one, so nothing is imposed.
108+
const whole = await driver.find('ticket', {} as any);
109+
expect(paged.map((r) => r.id)).toEqual(whole.map((r: any) => r.id));
110+
expect(paged.map((r) => r.id)).toEqual(PAGINATION_ROWS.map((r) => r.id));
111+
});
112+
}
70113
});

packages/plugins/driver-mongodb/src/mongodb-driver.ts

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ export class MongoDBDriver implements IDataDriver {
238238
}
239239

240240
// Sorting
241-
const sort = this.buildSortSpec(query.orderBy);
241+
const sort = this.buildSortSpec(query);
242242
if (sort) findOptions.sort = sort;
243243

244244
// Pagination
@@ -277,7 +277,7 @@ export class MongoDBDriver implements IDataDriver {
277277
projection: { _id: 0 },
278278
};
279279

280-
const sort = this.buildSortSpec(query.orderBy);
280+
const sort = this.buildSortSpec(query);
281281
if (sort) findOptions.sort = sort;
282282

283283
if (query.offset !== undefined) findOptions.skip = query.offset;
@@ -609,27 +609,46 @@ export class MongoDBDriver implements IDataDriver {
609609
* silently drops another — with every page full, every row real, and the two
610610
* halves of the symptom too far apart for anyone to notice.
611611
*
612+
* A paged read with **no** `sort` is the same defect at full strength
613+
* (objectstack#4363), which is why this reads the whole query rather than
614+
* just its `orderBy`. Unsorted documents come back in natural order, and
615+
* natural order describes where a document currently sits in its extent — it
616+
* moves when the document does, so page 2 of a walk can be cut from a layout
617+
* page 1 no longer describes. The empty sort key is simply the case where
618+
* every document ties with every other, so the same `id` suffix that was
619+
* separating one `status` group ends up carrying the entire order.
620+
*
612621
* `id` is always present (`create()` fills it when the caller omits one), so
613-
* unlike the SQL driver there is no table this cannot apply to. It is
614-
* appended in the LAST requested key's direction: determinism holds either
615-
* way, but a same-direction suffix is the one a compound index can still walk
616-
* in a single pass.
622+
* unlike the SQL driver there is no collection this cannot apply to, and
623+
* `syncCollectionSchema` gives every collection it provisions a unique
624+
* `idx_id_unique` — so a sort that ends in `id` is index-served rather than
625+
* a blocking in-memory sort against the 100 MB cap. It is appended in the
626+
* LAST requested key's direction (`1` when there is none): determinism holds
627+
* either way, but a same-direction suffix is the one a compound index can
628+
* still walk in a single pass.
617629
*
618-
* Returns `undefined` when nothing was requested — an unordered read stays
619-
* unordered (see the contract's explicit carve-out).
630+
* Returns `undefined` for a read that is neither sorted nor paged — nothing
631+
* is being sliced there, so a caller who asked for no order keeps none (the
632+
* contract's explicit carve-out).
620633
*/
621-
private buildSortSpec(orderBy: QueryAST['orderBy']): Document | undefined {
622-
if (!orderBy || !Array.isArray(orderBy)) return undefined;
634+
private buildSortSpec(query: QueryAST): Document | undefined {
623635
const sort: Document = {};
624636
let lastDirection: 1 | -1 = 1;
625-
for (const item of orderBy) {
626-
if (item.field) {
627-
lastDirection = item.order === 'desc' ? -1 : 1;
628-
sort[this.mapFieldName(item.field)] = lastDirection;
637+
if (Array.isArray(query.orderBy)) {
638+
for (const item of query.orderBy) {
639+
if (item.field) {
640+
lastDirection = item.order === 'desc' ? -1 : 1;
641+
sort[this.mapFieldName(item.field)] = lastDirection;
642+
}
629643
}
630644
}
631-
if (Object.keys(sort).length === 0) return undefined;
632-
if (sort.id === undefined) sort.id = lastDirection;
645+
646+
const requested = Object.keys(sort).length > 0;
647+
const paged = query.limit !== undefined || query.offset !== undefined;
648+
if (!requested && !paged) return undefined;
649+
650+
const idKey = this.mapFieldName('id');
651+
if (sort[idKey] === undefined) sort[idKey] = lastDirection;
633652
return sort;
634653
}
635654

0 commit comments

Comments
 (0)