fix(drivers): limit: 0 returns no records, on every driver and every read door (#6577)

.changeset/limit-zero-presence-sql-doors.md

@@ -0,0 +1,62 @@
+---
+'@objectstack/driver-sql': patch
+'@objectstack/driver-memory': patch
+'@objectstack/driver-mongodb': patch
+'@objectstack/driver-turso': patch
+'@objectstack/spec': minor
+---
+
+drivers: `limit: 0` returns no records, on every driver and every read door
+
+`limit: 0` was ruled in #6485 to mean **return no records**. Three of the five shipped
+drivers did not honour it, in three different ways — and the ones that disagreed
+returned **more** data than was requested, which on an ADR-0021 RLS read scope is
+over-reach rather than a loose filter. Reachable since #6578: the client now puts
+`top=0` on the wire, so the answer depended on which driver a deployment configured.
+
+**`driver-memory` — the slice was dropped.** `find()` sliced with `if (query.limit)`,
+truthiness, and `0` is falsy. Measured before the fix, three rows seeded:
+`{ limit: 0 }` returned **3 of 3**, and `{ limit: 0, offset: 1 }` returned 2 — the
+OFFSET applied and the LIMIT silently did not, which is why every paging suite stayed
+green over it. Two more sites of the same shape in `memory-analytics.ts` (the `$limit`
+pipeline stage and the SQL string builder) moved with it. Mingo honours `{ $limit: 0 }`
+as zero records (measured), so presence is sufficient there.
+
+**`driver-mongodb` — the value was forwarded faithfully, to a client that means
+something else by it.** `buildFindOptions` already tested presence, so `0` arrived
+exactly as written — but the MongoDB Node driver DEFINES `limit: 0` as *no limit*, so
+the answer was still the whole collection. Fixed with an explicit short-circuit that
+returns the empty result **before the client is consulted** (`[]` from `find`, `null`
+from `findOne`, which had the same hole). No round trip is made for a query whose
+answer is already known, and no future change in the upstream driver's reading of `0`
+can move this behaviour. Deliberately `=== 0`, not `<= 0`.
+
+**`driver-sql` — two doors disagreed with a third.** `findRows()`, the door `find()`
+goes through, has always compiled `limit` on presence. Two others compiled it on
+truthiness:
+
+- `findWithWindowFunctions()` — the live window-function read door (#4286). Returns
+  rows, so this was user-visible wrong data: `{ limit: 0 }` returned the whole table.
+- `analyzeQuery()` / `explain()` — returns a plan. It compiled `select * from "orders"`
+  where `find()` sent `... order by "id" asc limit ?`, so it explained a statement
+  other than the one that would run.
+
+`offset` moved with `limit` at both doors for internal consistency only. That half is
+**measured to change nothing**: knex elides a zero offset on better-sqlite3, Postgres
+and MySQL alike. It is pinned as the no-op it is rather than reported as a fix.
+
+**`driver-turso` remote transport — an `OFFSET` with no `LIMIT` was a syntax error.**
+Surfaced by the new conformance control that reads with a bare offset. SQLite's grammar
+is `LIMIT expr [OFFSET expr]`, and this compiler emitted the two clauses independently,
+so `find(obj, { offset: N })` with no `limit` produced `near "OFFSET": syntax error` —
+for **every** `N`, and only on the remote transport (the local half goes through knex,
+which synthesises the `LIMIT -1` no-limit sentinel). Remote now builds the same
+statement knex does.
+
+Result sets only ever get **narrower**. A caller who wants every row should omit
+`limit` rather than pass `0`.
+
+`@objectstack/spec` gains `PAGINATION_ZERO_LIMIT_CASES`, the shared conformance
+case-set pinning this — with controls, so "return nothing, always" cannot pass it. All
+**five** drivers answer it, with **no DEBT rows**: future drift goes red at
+`check:driver-conformance` rather than being discovered in production.

packages/drivers/driver-memory/src/memory-analytics.ts

@@ -428,10 +428,18 @@ export class MemoryAnalyticsService implements IAnalyticsService {
     }
 
     // Stage 6: $limit and $skip
+    //
+    // PRESENCE on the limit, not truthiness (#6577) — the same defect and the
+    // same reason as `memory-driver.ts`'s slice: `limit: 0` means "return no
+    // records" (#6485), `0` is falsy, so the stage was omitted entirely and an
+    // analytics read that asked for none came back with every row. Mingo
+    // honours `{ $limit: 0 }` as zero records (measured: 3 in, 0 out), so
+    // pushing the stage is sufficient here — no short-circuit needed, unlike
+    // the MongoDB driver, whose upstream client defines `0` as "no limit".
     if (query.offset) {
       pipeline.push({ $skip: query.offset });
     }
-    if (query.limit) {
+    if (query.limit !== undefined) {
       pipeline.push({ $limit: query.limit });
     }
 
@@ -595,7 +603,10 @@ export class MemoryAnalyticsService implements IAnalyticsService {
       );
       sql += ` ORDER BY ${orderClauses.join(', ')}`;
     }
-    if (query.limit) {
+    // PRESENCE, not truthiness (#6577) — the third site of the same shape in
+    // this package. `limit: 0` means "return no records" (#6485), and a dropped
+    // `LIMIT 0` widens the statement to the whole table.
+    if (query.limit !== undefined) {
       sql += ` LIMIT ${query.limit}`;
     }
     if (query.offset) {

packages/drivers/driver-memory/src/memory-driver.ts

@@ -312,7 +312,19 @@ export class InMemoryDriver implements IDataDriver {
     }
 
     // 4. Pagination (Limit)
-    if (query.limit) {
+    //
+    // PRESENCE, not truthiness (#6577). `limit: 0` means "return no records"
+    // (#6485), and `0` is falsy — so `if (query.limit)` dropped the slice and
+    // answered a request for NOTHING with the WHOLE table. Measured before this
+    // line changed, three rows seeded: `{ limit: 0 }` returned 3, and
+    // `{ limit: 0, offset: 1 }` returned 2 — the OFFSET applied and the LIMIT
+    // silently did not, which is why the shape survived every paging suite.
+    //
+    // `offset` above is deliberately left on truthiness: `slice(0)` IS the
+    // identity slice, so presence and truthiness cannot be told apart there —
+    // no behaviour to fix. The #5499 freeze exception granted here is the limit
+    // door only.
+    if (query.limit !== undefined) {
       results = results.slice(0, query.limit);
     }
 

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

@@ -35,6 +35,7 @@ import {
   PAGINATION_CASES,
   PAGINATION_ROWS,
   PAGINATION_UNORDERED_CASES,
+  PAGINATION_ZERO_LIMIT_CASES,
 } from '@objectstack/spec/data';
 import { InMemoryDriver } from './memory-driver.js';
 
@@ -110,4 +111,26 @@ describe('InMemoryDriver — paged reads are a partition of the result set (obje
       expect(paged.map((r) => r.id)).toEqual(PAGINATION_ROWS.map((r) => r.id));
     });
   }
+
+  /**
+   * `limit: 0` returns no records (#6485/#6577).
+   *
+   * This is the driver the card was filed about. `find()` sliced with
+   * `if (query.limit)` — truthiness — so `limit: 0` dropped the slice and the
+   * read that asked for nothing was answered with all twelve rows. Measured
+   * before the fix on a three-row table: `{ limit: 0 }` -> 3, and
+   * `{ limit: 0, offset: 1 }` -> 2, i.e. the OFFSET applied and the LIMIT did
+   * not — which is why every paging suite here stayed green over it.
+   *
+   * The #5499 investment freeze was lifted for this door specifically
+   * (maintainer ruling on #6577), and for nothing else in this package.
+   */
+  describe('`limit: 0` returns no records', () => {
+    for (const testCase of PAGINATION_ZERO_LIMIT_CASES) {
+      it(testCase.name, async () => {
+        const rows = await driver.find('ticket', { ...testCase.query });
+        expect(rows).toHaveLength(testCase.expectedRowCount);
+      });
+    }
+  });
 });

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

@@ -233,7 +233,34 @@ export class MongoDBDriver implements IDataDriver {
     return findOptions;
   }
 
+  /**
+   * `limit: 0` means **return no records** (#6485) — and this is the one driver
+   * where forwarding the value faithfully is not enough to say so.
+   *
+   * {@link buildFindOptions} already tests PRESENCE (`!== undefined`), so `0`
+   * reaches the client exactly as written. The divergence is one layer lower:
+   * the MongoDB Node driver DEFINES `limit: 0` as *no limit*, so a correctly
+   * forwarded `0` came back as the entire collection — the same wrong answer
+   * `driver-memory` gave for the opposite reason (it dropped the value; this
+   * one delivers it to a reader that means something else by it).
+   *
+   * So the contract is answered HERE rather than delegated: the empty result is
+   * returned without consulting the client at all. Two consequences worth being
+   * explicit about, because both are the point rather than side effects — no
+   * round trip is made for a query whose answer is already known, and no future
+   * change in the upstream driver's reading of `0` can move this behaviour.
+   *
+   * Deliberately `=== 0` and not `<= 0`: a negative limit is not a shape the
+   * contract defines, and quietly folding it into "no records" would invent an
+   * answer here instead of letting the layer that owns validation give one.
+   */
+  private returnsNoRecords(query: DriverQuery): boolean {
+    return query.limit === 0;
+  }
+
   async find(object: string, query: DriverQuery, options?: DriverOptions): Promise<Record<string, unknown>[]> {
+    if (this.returnsNoRecords(query)) return [];
+
     const collection = this.getCollection(object);
     const session = this.getSession(options);
 
@@ -246,6 +273,14 @@ export class MongoDBDriver implements IDataDriver {
   }
 
   async findOne(object: string, query: DriverQuery, options?: DriverOptions): Promise<Record<string, unknown> | null> {
+    // Same guard as `find()`, because this door has the same hole: `findOne`
+    // hands `query.limit` to the client through the SAME `buildFindOptions`, so
+    // `limit: 0` was read as "no limit" and answered with the first document —
+    // a record, where the contract says none. `null` is this signature's empty
+    // result. Leaving it out would have recreated, inside one driver, exactly
+    // the "one query, two answers" divergence this issue is about.
+    if (this.returnsNoRecords(query)) return null;
+
     const collection = this.getCollection(object);
     const session = this.getSession(options);
 

packages/drivers/driver-mongodb/src/mongodb-pagination-conformance.test.ts

@@ -31,6 +31,7 @@ import {
   PAGINATION_CASES,
   PAGINATION_ROWS,
   PAGINATION_UNORDERED_CASES,
+  PAGINATION_ZERO_LIMIT_CASES,
 } from '@objectstack/spec/data';
 import { MongoDBDriver } from './mongodb-driver.js';
 import { createTestMongod } from './test-mongod.js';
@@ -60,6 +61,21 @@ describe.skipIf(!sharedMongod)('driver-mongodb — paged reads are a partition o
     if (sharedMongod) await sharedMongod.stop();
   });
 
+  /**
+   * `limit: 0` returns no records (#6485/#6577) — the whole case-set, controls
+   * included, against a real mongod. The controls are what stop the
+   * short-circuit added for this contract from being an unconditional "return
+   * nothing": they read 2 and 12 rows back through the same door.
+   */
+  describe('`limit: 0` returns no records', () => {
+    for (const testCase of PAGINATION_ZERO_LIMIT_CASES) {
+      it(testCase.name, async () => {
+        const rows = await driver.find('ticket', { ...testCase.query });
+        expect(rows).toHaveLength(testCase.expectedRowCount);
+      });
+    }
+  });
+
   for (const testCase of PAGINATION_CASES) {
     it(`visits every row exactly once — ${testCase.name}`, async () => {
       const seen: string[] = [];
@@ -166,3 +182,46 @@ describe('MongoDBDriver — the sort spec sent to the server', () => {
     ).toEqual({ id: -1 });
   });
 });
+
+/**
+ * The `limit: 0` guard, on a driver that is NEVER CONNECTED — #6485/#6577.
+ *
+ * Outside the `skipIf` for the same reason the sort-spec block above is, and
+ * for one more that is specific to this contract: the guard's whole claim is
+ * that the answer is produced WITHOUT consulting the MongoDB client. A test
+ * that needs a running mongod to prove that would be asserting the opposite of
+ * what it is about. Here the URI is unreachable by construction (port 1), so if
+ * the short-circuit were removed these cases would not fail on a row count —
+ * they would fail on a connection error, which is exactly the right alarm.
+ *
+ * Why this driver needed a guard at all when its `buildFindOptions` was already
+ * correct: it tests presence (`!== undefined`), so `0` was forwarded faithfully
+ * — into a client that DEFINES `limit: 0` as *no limit*. Forwarding was the
+ * problem, not the fix.
+ */
+describe('MongoDBDriver — `limit: 0` is answered before the client is consulted', () => {
+  const driver = new MongoDBDriver({ url: 'mongodb://127.0.0.1:1/unused', database: 'unused' });
+
+  it('find() returns zero records without dialing the server', async () => {
+    await expect(driver.find('ticket', { limit: 0 })).resolves.toHaveLength(0);
+  });
+
+  it('find() returns zero records with an offset too', async () => {
+    await expect(driver.find('ticket', { limit: 0, offset: 5 })).resolves.toHaveLength(0);
+  });
+
+  it('findOne() returns null — the empty result for its signature', async () => {
+    await expect(driver.findOne('ticket', { limit: 0 })).resolves.toBeNull();
+  });
+
+  it('does NOT short-circuit a non-zero limit — that read still needs the server', async () => {
+    // The control that keeps the guard from becoming "return nothing, always".
+    // With no short-circuit the unreachable URI is what answers, so a rejection
+    // here IS the assertion: the call went to the client, as it must.
+    await expect(driver.find('ticket', { limit: 2 })).rejects.toThrow();
+  });
+
+  it('does NOT short-circuit a read with no limit at all', async () => {
+    await expect(driver.find('ticket', {})).rejects.toThrow();
+  });
+});