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
17 changes: 17 additions & 0 deletions .changeset/22790-search-total-counted.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
"@objectstack/spec": minor
"@objectstack/objectql": minor
"@objectstack/metadata-protocol": patch
---

A paged list under a search now reports the real number of matching records as `total`, and `hasMore` agrees with it on every page.

Clause-②: yes (widening) — `EngineCountOptionsSchema` gains two OPTIONAL keys, `search` and `searchFields`, so the accept set of the count options grows. Nothing previously admitted is refused, no key is renamed or retired, and no producer is required to write them.

Until now `GET /api/v1/data/:object?$top=25&$search=Line` over 75 matching records answered `total: 26` (offset + page + 1, a page-local estimate), and the last page claimed `hasMore: true`; `POST /api/v1/data/:object/query` with a `search` did the same. Nothing on the response marked the number as an estimate, so the Console rendered it as the record count and offered the wrong number of pages.

- **`@objectstack/spec`** — `EngineCountOptionsSchema` declares `search` (the bare string, or the structured `FullTextSearchSchema` form) and `searchFields`, identically to `EngineQueryOptionsSchema`. FROM `count(object, { where })` only → TO `count(object, { where, search, searchFields })`. The structured `search` form carries flag defaults, so the parsed state is named `EngineCountOptionsParsed` (ADR-0122). `HookCountQuery` (`ctx.api.object(name).count()`) derives from it and gains the same two keys.
- **`@objectstack/objectql`** — `engine.count()` (and `ctx.api.object(name).count()`) accepts the two keys it used to refuse as unknown options, and expands them through the same ADR-0061 expansion `find()` and `aggregate()` use: the same server-resolved searchable fields, the same `internal: true` exclusion, the same `searchFields` narrowing, AND-ed with `where` before the security middlewares run. There is one expander, not two, so a count under a search is the number of rows `find()` returns for the same bag.
- **`@objectstack/metadata-protocol`** — `findData`'s paged branch passes `search` / `searchFields` to `engine.count()`, and the page-local estimate branch is gone. `$count=false` still skips the count and omits `total`.

Nothing to migrate. A client that worked around the estimate, for example by labelling the count as page-local under a search, can read `total` as the match count again.
23 changes: 21 additions & 2 deletions content/docs/references/data/data-engine.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -221,14 +221,16 @@ Options for DataEngine.count operations
| :--- | :--- | :--- | :--- |
| **method** | `'count'` | ✅ | |
| **object** | `string` | ✅ | |
| **query** | `{ context?: object; where?: Record<string, any> \| any; filter?: Record<string, any> \| any }` | optional | |
| **query** | `{ context?: object; where?: Record<string, any> \| any; search?: string \| object; searchFields?: string[]; … }` | optional | |

### Nested Shape: `DataEngineCountRequest.query`

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
| **where** | `Record<string, any> \| any` | optional | |
| **search** | `string \| { query: string; fields?: string[]; fuzzy?: boolean; operator?: Enum<'and' \| 'or'>; … }` | optional | |
| **searchFields** | `string[]` | optional | |
| **filter** | `Record<string, any> \| any` | optional | Data Engine query filter conditions |


Expand Down Expand Up @@ -684,14 +686,16 @@ This schema accepts one of the following structures:
| :--- | :--- | :--- | :--- |
| **method** | `'count'` | ✅ | |
| **object** | `string` | ✅ | |
| **query** | `{ context?: object; where?: Record<string, any> \| any; filter?: Record<string, any> \| any }` | optional | |
| **query** | `{ context?: object; where?: Record<string, any> \| any; search?: string \| object; searchFields?: string[]; … }` | optional | |

### Nested Shape: `DataEngineRequest[method='count'].query`

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
| **where** | `Record<string, any> \| any` | optional | |
| **search** | `string \| { query: string; fields?: string[]; fuzzy?: boolean; operator?: Enum<'and' \| 'or'>; … }` | optional | |
| **searchFields** | `string[]` | optional | |
| **filter** | `Record<string, any> \| any` | optional | Data Engine query filter conditions |

---
Expand Down Expand Up @@ -994,6 +998,8 @@ QueryAST-aligned options for DataEngine.count operations
| :--- | :--- | :--- | :--- |
| **context** | `{ userId?: string; actor?: string; attributedUserId?: string; email?: string; … }` | optional | |
| **where** | `Record<string, any> \| any` | optional | |
| **search** | `string \| { query: string; fields?: string[]; fuzzy?: boolean; operator?: Enum<'and' \| 'or'>; … }` | optional | |
| **searchFields** | `string[]` | optional | |

### Nested Shape: `EngineCountOptions.context`

Expand Down Expand Up @@ -1033,6 +1039,19 @@ QueryAST-aligned options for DataEngine.count operations
| **transaction** | `any` | optional | |
| **traceId** | `string` | optional | |

### Nested Shape: `EngineCountOptions.search`

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **query** | `string` | ✅ | Search query text |
| **fields** | `string[]` | optional | Fields to search in (if not specified, searches all text fields) |
| **fuzzy** | `boolean` | optional (default: `false`) | [EXPERIMENTAL — not enforced] Fuzzy matching (tolerate typos). The ADR-0061 expansion reads only `query` + `fields`; no executor receives this flag. |
| **operator** | `Enum<'and' \| 'or'>` | optional (default: `"or"`) | [EXPERIMENTAL — not enforced] Logical operator between terms. The ADR-0061 expansion applies its own term semantics; no executor receives this flag. |
| **boost** | `Record<string, number>` | optional | [EXPERIMENTAL — not enforced] Field-specific relevance boosting (field name -> boost factor). No executor scores results. |
| **minScore** | `number` | optional | [EXPERIMENTAL — not enforced] Minimum relevance score threshold. No executor scores results. |
| **language** | `string` | optional | [EXPERIMENTAL — not enforced] Language for text analysis (e.g., "en", "zh", "es"). No executor selects an analyzer. |
| **highlight** | `boolean` | optional (default: `false`) | [EXPERIMENTAL — not enforced] Search result highlighting. No executor emits highlights. |


---

Expand Down
33 changes: 14 additions & 19 deletions packages/metadata-protocol/src/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13292,10 +13292,11 @@ export class ObjectStackProtocolImplementation implements
// whether more pages remain (true server-side pagination). Without a limit
// the full result set is returned, so its length already IS the total.
//
// engine.count() only honors `where`; a `search`/`distinct` query can't be
// reproduced by it, so for those we skip the count and fall back to a
// page-local estimate (a full page implies there may be more) rather than
// reporting a wrong total.
// [#22790] The count takes the query's `search` / `searchFields` too: the
// engine expands them through the one ADR-0061 expander `find` uses, so
// `total` counts exactly the rows this query pages. A search used to skip
// the count and report a page-local estimate (`offset + page + 1`) that
// the Console rendered as the record count.
//
// [#22588] `limit: 0` IS a limit. It asks for no rows (objectstack#6485,
// the drivers' pagination conformance), so `records.length` is 0 and was
Expand All @@ -13306,9 +13307,8 @@ export class ObjectStackProtocolImplementation implements
// the full set is the total" arm, which is true of every limit except
// this one. So `>= 0`: a zero-row request is a paged request like any
// other, and the three arms below keep their meaning for it — the count
// runs (`total` is the filtered count, whatever the page size), `$count=false`
// still omits `total` instead of reporting the page's 0, and `search`
// still reports its page-local estimate rather than a count.
// runs (`total` is the filtered count, whatever the page size), and
// `$count=false` still omits `total` instead of reporting the page's 0.
const pageLimit = typeof options.limit === 'number' && options.limit >= 0 ? options.limit : undefined;
const pageOffset = typeof options.offset === 'number' && options.offset > 0 ? options.offset : 0;
let total: number | undefined = records.length;
Expand All @@ -13318,7 +13318,6 @@ export class ObjectStackProtocolImplementation implements
// the flag's ONLY observable effect platform-wide, on a capability
// that never deduplicated a row. Removed with `query.distinct`
// (tombstoned in spec 17); `total`/`hasMore` are truthful again.
const countable = options.search == null;
if (countOptOut) {
// [#10757] The caller said it does not need `total`, so the
// COUNT query is not issued at all — that is the whole point of
Expand All @@ -13330,19 +13329,16 @@ export class ObjectStackProtocolImplementation implements
// filter (IF REQUESTED)",
// `packages/spec/src/api/protocol.zod.ts`), so absent is the
// declared shape for "not requested" — and it is the only
// honest one: the `search` branch below reports an estimate
// because it has no better number to give, while here a real
// number was available and the caller declined it. Handing back
// a plausible-looking guess under those circumstances is how a
// honest one: a real number was available and the caller
// declined it. Handing back a plausible-looking guess is how a
// page-local estimate ends up rendered as a record count.
//
// `hasMore` is still answered, from the page alone: a FULL page
// means there may be more. Same page-local rule the search
// branch uses, and it never over-reports the data — it can only
// say "maybe more" on an exactly-full last page.
// means there may be more. It never over-reports the data — it
// can only say "maybe more" on an exactly-full last page.
hasMore = records.length === pageLimit;
total = undefined;
} else if (countable) {
} else {
// [#10757] `counted` is a separate, always-assigned local so
// `hasMore` below compares against a `number`: `total` became
// optional when `$count=false` gained the right to omit it, and
Expand All @@ -13351,6 +13347,8 @@ export class ObjectStackProtocolImplementation implements
try {
counted = await this.engine.count(request.object, {
where: options.where,
search: options.search,
searchFields: options.searchFields,
context: options.context,
} as any);
} catch {
Expand All @@ -13360,9 +13358,6 @@ export class ObjectStackProtocolImplementation implements
}
total = counted;
hasMore = pageOffset + records.length < counted;
} else {
hasMore = records.length === pageLimit;
total = pageOffset + records.length + (hasMore ? 1 : 0);
}
}
return {
Expand Down
14 changes: 8 additions & 6 deletions packages/metadata-protocol/src/protocol.zero-limit-total.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@
* count for a zero-row page would pass (1) and fail here.
* 3. The two neighbouring arms keep their meaning for a zero-row page:
* `$count=false` still issues no COUNT and omits `total` (it no longer
* reports the page's `0` as if it were a count), and `search` still issues
* no COUNT — `engine.count()` cannot reproduce a search, which is why that
* arm reports a page-local estimate at every page size.
* reports the page's `0` as if it were a count), and a `search` is counted
* like any other filter (#22790: the COUNT carries the search the rows
* were read with).
* 4. Absent a limit nothing moves: no COUNT, `total` is the set's length.
*/

Expand Down Expand Up @@ -149,10 +149,12 @@ describe('[#22588] the neighbouring arms keep their meaning for a zero-row page'
expect('total' in result).toBe(false);
});

it('search still issues no COUNT — engine.count() cannot reproduce a search', async () => {
const { result, count } = await list({ $top: '0', $search: 't' });
it('a search is counted, with the search the rows were read with (#22790)', async () => {
const { result, count, find } = await list({ $top: '0', $search: 't' });

expect(count).not.toHaveBeenCalled();
expect(count).toHaveBeenCalledTimes(1);
expect((count.mock.calls[0] as unknown[])[1]).toMatchObject({ search: 't' });
expect((find.mock.calls[0] as unknown[])[1]).toMatchObject({ search: 't' });
expect(result.records).toEqual([]);
});

Expand Down
44 changes: 24 additions & 20 deletions packages/objectql/src/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -619,12 +619,13 @@ const ENGINE_DELETE_OPTION_KEYS: ReadonlySet<string> = new Set([
'context', 'where', 'multi',
...ENGINE_DRIVER_PASSTHROUGH_KEYS,
]);
const ENGINE_COUNT_OPTION_KEYS: ReadonlySet<string> = new Set(['context', 'where']);
// `search` / `searchFields` are READ by `aggregate` through the same ADR-0061
// expander `find` uses ({@link ObjectQL.expandSearchOnAggregateOptions}), so a
// grouped answer under a search is the grouping of the searched rows. Before
// `EngineAggregateOptionsSchema` declared them they were refused here, and the
// one wire path to this verb left them out — the unsearched groups answered.
// `search` / `searchFields` are READ by `count` and `aggregate` through the
// same ADR-0061 expander `find` uses ({@link ObjectQL.expandSearchOnOptions}),
// so a count or a grouped answer under a search is over the searched rows.
// Before their schemas declared them they were refused here, and the wire paths
// left them out: the unsearched groups answered, and a paged list's `total`
// under a search was a page-local estimate (#22790).
const ENGINE_COUNT_OPTION_KEYS: ReadonlySet<string> = new Set(['context', 'where', 'search', 'searchFields']);
const ENGINE_AGGREGATE_OPTION_KEYS: ReadonlySet<string> = new Set([
'context', 'where', 'groupBy', 'aggregations', 'having', 'timezone',
'search', 'searchFields',
Expand Down Expand Up @@ -12679,13 +12680,15 @@ export class ObjectQL implements IObjectQLEngine {
}

/**
* The aggregate verb's door into {@link expandSearchOnAst} — ⛔ not a second
* expander. `QuerySchema.search` (ADR-0061 D1) is declared beside `groupBy` /
* `aggregations` with no carve-out, so a grouped read under a search groups
* the SEARCHED rows: the same cross-field `$or`, resolved from the same
* server-side searchable set and the same `searchFields` narrowing, ANDed
* with `where` exactly as on `find` — before the security middlewares run,
* so an injected RLS predicate composes with it the same way on both verbs.
* The aggregate and count verbs' door into {@link expandSearchOnAst} — ⛔ not
* a second expander. `QuerySchema.search` (ADR-0061 D1) is declared beside
* `groupBy` / `aggregations` with no carve-out, so a grouped read under a
* search groups the SEARCHED rows, and a count under one counts them (#22790:
* `findData`'s paged `total`): the same cross-field `$or`, resolved from the
* same server-side searchable set, `internal: true` exclusion and
* `searchFields` narrowing, ANDed with `where` exactly as on `find` — before
* the security middlewares run, so an injected RLS predicate composes with it
* the same way on every verb.
*
* The expansion runs on a carrier AST holding only the three keys it reads,
* and the result is a COPY of `query` with `where` replaced and the two
Expand All @@ -12694,16 +12697,13 @@ export class ObjectQL implements IObjectQLEngine {
* `opCtx.options` unexpanded. The common path — no search key — returns the
* same reference and allocates nothing.
*/
private expandSearchOnAggregateOptions(
object: string,
query: EngineAggregateOptions,
): EngineAggregateOptions {
private expandSearchOnOptions<T extends EngineAggregateOptions | EngineCountOptions>(object: string, query: T): T {
if (query.search === undefined && query.searchFields === undefined) return query;
const { search, searchFields, ...rest } = query;
const carrier = { object, where: rest.where, search, searchFields } as QueryAST;
this.expandSearchOnAst(carrier, this._registry.getObject(object));
if (carrier.where === undefined) return rest;
return { ...rest, where: carrier.where };
if (carrier.where === undefined) return rest as T;
return { ...rest, where: carrier.where } as T;
}

/**
Expand Down Expand Up @@ -18695,6 +18695,10 @@ export class ObjectQL implements IObjectQLEngine {
query = foldEngineOptionAliases(object, 'count', query, ENGINE_WHERE_SLOTS);
rejectUnknownEngineOptions(object, 'count', query, ENGINE_COUNT_OPTION_KEYS);
query = lowerWhereFilterArray(object, 'count', query, this._registry.getObject(object), this.relatedSchemaOf);
// [#22790] ADR-0061 `search` → the rows are searched before they are
// counted, by the one expander `find` runs, at the same point in the
// sequence — so the count is the number of rows `find` returns for the bag.
if (query) query = this.expandSearchOnOptions(object, query);
const driver = this.getDriver(object);

// The AST must ride on the opCtx so the security/sharing middlewares can
Expand Down Expand Up @@ -18830,7 +18834,7 @@ export class ObjectQL implements IObjectQLEngine {
// ADR-0061 `search` → the rows are searched BEFORE they are grouped, by
// the one expander `find` runs, at the same point in the sequence (after
// the `where` doors above, before the AST is built and tokens resolve).
query = this.expandSearchOnAggregateOptions(object, query);
query = this.expandSearchOnOptions(object, query);
this.rejectCredentialAggregation(object, query);
// [#20783] …and a `groupBy` entry naming a structured-JSON field (`json`,
// `composite`, `address`, …) is refused `INVALID_FIELD` / 400 here, before
Expand Down
10 changes: 6 additions & 4 deletions packages/objectql/src/protocol-data.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,14 +287,16 @@ describe('ObjectStackProtocolImplementation - Data Operations', () => {
expect(result.hasMore).toBe(false);
});

it('skips count for search queries and estimates hasMore from a full page', async () => {
// engine.count() can't reproduce a $search, so we must not call it; a
// full page (length === limit) implies there may be more.
it('counts a search query under the same search, and reports that total (#22790)', async () => {
// The engine expands `search` through the one expander `find` uses,
// so the total is a count, never a page-local estimate.
mockEngine.find.mockResolvedValue(Array.from({ length: 10 }, (_, i) => ({ id: `r${i}` })));
mockEngine.count.mockResolvedValue(75);

const result = await protocol.findData({ object: 'task', query: { $top: 10, $search: 'foo' } });

expect(mockEngine.count).not.toHaveBeenCalled();
expect(mockEngine.count).toHaveBeenCalledWith('task', expect.objectContaining({ search: 'foo' }));
expect(result.total).toBe(75);
expect(result.hasMore).toBe(true);
});
});
Expand Down
Loading
Loading