Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions .changeset/paged-read-determinism.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
---
"@objectstack/spec": patch
"@objectstack/driver-sql": patch
"@objectstack/driver-mongodb": patch
---

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". So page 2 could repeat a row page 1 already showed and skip one nobody
ever saw:

```
page 1: ORDER BY status LIMIT 5 OFFSET 0 -> [r05 r07 r11 r04 …]
page 2: ORDER BY status LIMIT 5 OFFSET 5 -> [r04 …] r04 again; one row never served
```

Every page is full, every row is real and belongs, and the duplicate sits
several screens from the omission — which is why this is found by a user
counting records, never by reading a response.

`SqlDriver` and `MongoDBDriver` now 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 one pass).
`driver-memory` already conformed — `Array#sort` is stable over a table whose
order does not move — and now has a suite saying so, because that property is
implicit and easy to lose in a refactor that looks like a speed-up.

`SqlDriver` adds it only for objects it created itself (`initObjects` records
those). A federated table (ADR-0015) may have no `id` column, and guessing there
would be worse than doing nothing: the unknown-column error is answered by
#3821's ladder retrying with **no ORDER BY at all**, trading a reshuffle among
ties for the loss of the caller's whole sort.

The obligation is now normative on `IDataDriver.find`, with shared cases in
`@objectstack/spec/data` (`PAGINATION_CASES`) that all three drivers run — so a
future driver is held to it by a gate rather than by remembering.

Deliberately not covered: a paged read with **no** `orderBy`. That is
non-deterministic on every backend by definition and imposing an order on
callers who asked for none changes plan selection far more broadly; filed as
#4363.
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* Deterministic paged reads for the in-memory driver (objectui#3106) — the
* contract on `IDataDriver.find`, checked against the shared cases in
* `@objectstack/spec/data`.
*
* This driver needed **no change** to satisfy it, and that is worth a suite
* rather than a shrug. It sorts with `Array#sort`, which ES2019 onward
* guarantees is stable, over a table array whose order does not move between
* two reads — so equal keys keep the same relative arrangement on page 2 that
* they had on page 1, which is precisely what the contract asks for.
*
* The guarantee is therefore load-bearing but implicit: it rests on `sort`
* being stable and on `applySort` copying rather than reordering the table in
* place. Both are easy to lose in a refactor that looks like a speed-up — a
* hand-rolled quicksort, or sorting the backing array directly — and neither
* loss would fail any other test in this package. That is what this file is
* for: it holds the property against the day the implementation changes, which
* is the only day it could break.
*/

import { describe, it, expect, beforeEach } from 'vitest';
import { PAGINATION_ALL_IDS, PAGINATION_CASES, PAGINATION_ROWS } from '@objectstack/spec/data';
import { InMemoryDriver } from './memory-driver.js';

describe('InMemoryDriver — paged reads are a partition of the result set (objectui#3106)', () => {
let driver: InMemoryDriver;

beforeEach(async () => {
driver = new InMemoryDriver({ persistence: false });
await driver.connect();
for (const row of PAGINATION_ROWS) {
await driver.create('ticket', { ...row });
}
});

for (const testCase of PAGINATION_CASES) {
it(`visits every row exactly once — ${testCase.name}`, async () => {
const seen: string[] = [];
for (let offset = 0; offset < PAGINATION_ROWS.length; offset += testCase.pageSize) {
const page = await driver.find('ticket', {
orderBy: [...testCase.orderBy],
limit: testCase.pageSize,
offset,
} as any);
seen.push(...page.map((r: any) => String(r.id)));
}

expect(seen).toHaveLength(PAGINATION_ALL_IDS.length);
expect(new Set(seen).size).toBe(PAGINATION_ALL_IDS.length);
expect([...seen].sort()).toEqual([...PAGINATION_ALL_IDS].sort());
});

it(`page boundaries are invisible — ${testCase.name}`, async () => {
const paged: any[] = [];
for (let offset = 0; offset < PAGINATION_ROWS.length; offset += testCase.pageSize) {
const page = await driver.find('ticket', {
orderBy: [...testCase.orderBy],
limit: testCase.pageSize,
offset,
} as any);
paged.push(...page);
}

const whole = await driver.find('ticket', { orderBy: [...testCase.orderBy] } as any);
expect(paged.map((r) => r.id)).toEqual(whole.map((r: any) => r.id));
});
}
});
58 changes: 40 additions & 18 deletions packages/plugins/driver-mongodb/src/mongodb-driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,15 +238,8 @@ export class MongoDBDriver implements IDataDriver {
}

// Sorting
if (query.orderBy && Array.isArray(query.orderBy)) {
const sort: Document = {};
for (const item of query.orderBy) {
if (item.field) {
sort[this.mapFieldName(item.field)] = item.order === 'desc' ? -1 : 1;
}
}
findOptions.sort = sort;
}
const sort = this.buildSortSpec(query.orderBy);
if (sort) findOptions.sort = sort;

// Pagination
if (query.offset !== undefined) findOptions.skip = query.offset;
Expand Down Expand Up @@ -284,15 +277,8 @@ export class MongoDBDriver implements IDataDriver {
projection: { _id: 0 },
};

if (query.orderBy && Array.isArray(query.orderBy)) {
const sort: Document = {};
for (const item of query.orderBy) {
if (item.field) {
sort[this.mapFieldName(item.field)] = item.order === 'desc' ? -1 : 1;
}
}
findOptions.sort = sort;
}
const sort = this.buildSortSpec(query.orderBy);
if (sort) findOptions.sort = sort;

if (query.offset !== undefined) findOptions.skip = query.offset;
if (query.limit !== undefined) findOptions.limit = query.limit;
Expand Down Expand Up @@ -611,6 +597,42 @@ export class MongoDBDriver implements IDataDriver {
return field;
}

/**
* The `sort` spec for a `find`, with a unique tie-breaker appended so that
* paging is a partition of the result set rather than a series of unrelated
* queries (objectui#3106, contract on `IDataDriver.find`).
*
* MongoDB is explicit that this is not free: `sort` on a non-unique key
* combined with `skip`/`limit` "may return the same document more than once"
* because equal keys have no defined relative order and nothing holds that
* order steady between two executions. Page 2 repeats a row from page 1 and
* silently drops another — with every page full, every row real, and the two
* halves of the symptom too far apart for anyone to notice.
*
* `id` is always present (`create()` fills it when the caller omits one), so
* unlike the SQL driver there is no table this cannot apply to. It is
* appended in the LAST requested key's direction: determinism holds either
* way, but a same-direction suffix is the one a compound index can still walk
* in a single pass.
*
* Returns `undefined` when nothing was requested — an unordered read stays
* unordered (see the contract's explicit carve-out).
*/
private buildSortSpec(orderBy: QueryAST['orderBy']): Document | undefined {
if (!orderBy || !Array.isArray(orderBy)) return undefined;
const sort: Document = {};
let lastDirection: 1 | -1 = 1;
for (const item of orderBy) {
if (item.field) {
lastDirection = item.order === 'desc' ? -1 : 1;
sort[this.mapFieldName(item.field)] = lastDirection;
}
}
if (Object.keys(sort).length === 0) return undefined;
if (sort.id === undefined) sort.id = lastDirection;
return sort;
}

// ── Temporal storage form (#4047) ─────────────────────────────────────────

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* Deterministic paged reads for the MongoDB driver (objectui#3106) — the
* contract on `IDataDriver.find`, checked against the shared cases in
* `@objectstack/spec/data` against a real MongoDB via `mongodb-memory-server`.
*
* This is the backend where the defect is not theoretical. MongoDB documents
* that `sort` on a non-unique key combined with `skip`/`limit` may return the
* same document more than once — equal keys have no defined relative order, and
* nothing holds one execution's arrangement steady for the next. So the driver
* appends `id` to every requested sort, and these cases walk the page
* boundaries that would otherwise be where a row is served twice while another
* is never served at all.
*
* The sort-spec assertions at the end are deliberately about the spec object
* rather than the rows: they are what fails if the tie-breaker is removed on a
* day the fixture happens to come back in a stable order anyway.
*/

import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { MongoMemoryServer } from 'mongodb-memory-server';
import { PAGINATION_ALL_IDS, PAGINATION_CASES, PAGINATION_ROWS } from '@objectstack/spec/data';
import { MongoDBDriver } from './mongodb-driver.js';

let sharedMongod: MongoMemoryServer | undefined;
try {
sharedMongod = await MongoMemoryServer.create({ instance: { launchTimeout: 60_000 } });
} catch (err) {
console.warn(
'[driver-mongodb] Skipping pagination conformance — mongodb-memory-server could not start: ' +
`${(err as Error)?.message ?? String(err)}`,
);
}

describe.skipIf(!sharedMongod)('driver-mongodb — paged reads are a partition of the result set', () => {
const mongod = sharedMongod as MongoMemoryServer;
let driver: MongoDBDriver;

beforeAll(async () => {
driver = new MongoDBDriver({ url: mongod.getUri(), database: 'pagination_conformance' });
await driver.connect();
for (const row of PAGINATION_ROWS) {
await driver.create('ticket', { ...row });
}
}, 90_000);

afterAll(async () => {
if (driver) await driver.disconnect();
if (sharedMongod) await sharedMongod.stop();
});

for (const testCase of PAGINATION_CASES) {
it(`visits every row exactly once — ${testCase.name}`, async () => {
const seen: string[] = [];
for (let offset = 0; offset < PAGINATION_ROWS.length; offset += testCase.pageSize) {
const page = await driver.find('ticket', {
orderBy: [...testCase.orderBy],
limit: testCase.pageSize,
offset,
} as any);
seen.push(...page.map((r) => String(r.id)));
}

expect(seen).toHaveLength(PAGINATION_ALL_IDS.length);
expect(new Set(seen).size).toBe(PAGINATION_ALL_IDS.length);
expect([...seen].sort()).toEqual([...PAGINATION_ALL_IDS].sort());
});

it(`page boundaries are invisible — ${testCase.name}`, async () => {
const paged: any[] = [];
for (let offset = 0; offset < PAGINATION_ROWS.length; offset += testCase.pageSize) {
const page = await driver.find('ticket', {
orderBy: [...testCase.orderBy],
limit: testCase.pageSize,
offset,
} as any);
paged.push(...page);
}

const whole = await driver.find('ticket', { orderBy: [...testCase.orderBy] } as any);
expect(paged.map((r) => r.id)).toEqual((whole as any[]).map((r) => r.id));
});
}

it('leaves an unordered read alone — no sort is imposed on a caller who asked for none', () => {
expect(driver['buildSortSpec'](undefined)).toBeUndefined();
expect(driver['buildSortSpec']([])).toBeUndefined();
});

it('appends `id` in the LAST key\'s direction', () => {
expect(driver['buildSortSpec']([{ field: 'status', order: 'asc' }])).toEqual({
status: 1,
id: 1,
});
expect(
driver['buildSortSpec']([
{ field: 'status', order: 'asc' },
{ field: 'rank', order: 'desc' },
]),
).toEqual({ status: 1, rank: -1, id: -1 });
});

it('does not override `id` when the caller already sorted by it', () => {
expect(driver['buildSortSpec']([{ field: 'id', order: 'desc' }])).toEqual({ id: -1 });
});
});
Loading
Loading