diff --git a/.changeset/query-cursor-removed.md b/.changeset/query-cursor-removed.md new file mode 100644 index 0000000000..ba9e9caa99 --- /dev/null +++ b/.changeset/query-cursor-removed.md @@ -0,0 +1,33 @@ +--- +"@objectstack/spec": major +"@objectstack/client": major +--- + +refactor(data)!: `query.cursor` is removed — no driver ever implemented keyset pagination (#4286 step 4) + +`cursor` promised keyset pagination and nothing served it: the key was accepted +and ignored, so every page came back identical — a caller looping "until +`hasMore` is false" never terminated. It was Tier A of the #4286 inventory: a +shipped public producer (`QueryBuilder.cursor()`) minting a key no executor +read. + +**FROM → TO** + +| Was | Now | +| :--- | :--- | +| `cursor: { created_at: last.created_at }` | `where: { created_at: { $gt: last.created_at } }` + the matching `orderBy` | +| `QueryBuilder.cursor({...})` | `.where({ created_at: { $gt: ... } }).orderBy('created_at')` | + +The one-line fix: **delete the key and seek with `where` on your sort key** — +every driver already executes that, with canonicalised temporal comparands. + +Mechanics: `retiredKey()` tombstones on both declaration sites +(`QuerySchema.cursor` and `EngineQueryOptionsSchema.cursor`, one shared +prescription), so authoring the key fails `tsc` and a query still carrying it +fails to parse with the fix. `QueryBuilder.cursor()` is deleted. Registered as +the protocol-18 semantic migration `query-cursor-retired` (request surface — +nothing stored to rewrite). The caller-built `Record` shape +would not survive a real keyset design anyway: a first-class cursor, if ever +built, will be a response-minted opaque token (the pattern the +metadata-revision / flow-run / notification list endpoints already use — those +`cursor` params are unrelated and unchanged). diff --git a/.changeset/query-distinct-removed.md b/.changeset/query-distinct-removed.md new file mode 100644 index 0000000000..ef6bd153f1 --- /dev/null +++ b/.changeset/query-distinct-removed.md @@ -0,0 +1,37 @@ +--- +"@objectstack/spec": major +"@objectstack/client": major +"@objectstack/metadata-protocol": minor +--- + +refactor(data)!: `query.distinct` is removed, and with it the mis-wired REST count suppression (#4286 step 4) + +`distinct` promised `SELECT DISTINCT` and no driver ever rendered it — but it +was **mis-wired rather than merely dead** (#4286 finding 2, the harsher +ADR-0078 class): its only observable effect platform-wide was that the REST +list path treated a distinct query as *not countable*, silently degrading +`total`/`hasMore` to a page-local estimate while still returning duplicate +rows. A caller — or a self-verifying agent — saw the response change and +concluded the flag worked. It had a shipped public producer +(`QueryBuilder.distinct()`). + +**FROM → TO** + +| Was | Now | +| :--- | :--- | +| `distinct: true` for unique combinations | `groupBy: ['category']` | +| `distinct: true` + count | `aggregations: [{ function: 'count_distinct', field: 'category', alias: '...' }]` | +| one column's distinct values | the SQL/memory drivers' `distinct(object, field)` door (driver-level) | + +The one-line fix: **delete the key**; deduplicate with `groupBy` / +`count_distinct`. + +Mechanics: `retiredKey()` tombstones on both declaration sites +(`QuerySchema.distinct` and `EngineQueryOptionsSchema.distinct`, one shared +prescription); `QueryBuilder.distinct()` is deleted; registered as the +protocol-18 semantic migration `query-distinct-retired`. **Observable REST +change (`@objectstack/metadata-protocol`):** the count-suppression branch is +deleted — a list request that used to carry `distinct` now gets a real +`total`/`hasMore` again (that restoration is the point, not a side effect). +The per-aggregation `distinct` flag (`AggregationNode.distinct`) is a +different, live member and is untouched. diff --git a/.changeset/query-having-enforced.md b/.changeset/query-having-enforced.md new file mode 100644 index 0000000000..34c2cfde9b --- /dev/null +++ b/.changeset/query-having-enforced.md @@ -0,0 +1,39 @@ +--- +"@objectstack/spec": minor +"@objectstack/objectql": minor +"@objectstack/metadata-protocol": minor +--- + +feat(objectql)!: `query.having` is enforced — the engine applies it after aggregation (#4286 step 3, ADR-0049 resolved to enforce) + +`having` had been declared on the request surface since AST v2 and executed by +nothing. #4286 finding 1 showed the gap was structural: `engine.aggregate()` +rebuilt the driver AST with exactly `object`/`where`/`groupBy`/`aggregations`, +so even a driver that *did* implement HAVING could never have received it, and +the one wire path (`findData`'s aggregate branch) dropped the clause too. It +was the strongest enforce candidate of the #4286 set — the clause every +SQL-literate author (human or model) expects to work next to +`groupBy`/`aggregations` — and it is now live end to end: + +- **Engine-owned, both paths.** `applyHaving()` + (`packages/objectql/src/having-filter.ts`) runs AFTER aggregation on the + native-driver path and the in-memory fallback alike — the same + correct-first / optimize-later two-tier shape date bucketing uses. Native + SQL `HAVING` pushdown can come later behind a driver capability flag without + changing semantics. +- **Namespace: the aggregated row's own columns** — aggregation aliases + (`order_count`, `total`) and groupBy projections — with the ordinary + FilterCondition operators plus `$and`/`$or`/`$not`. +- **An unknown operator rejects loudly.** Ignoring one (as tolerant matchers + do) would silently return unfiltered aggregates — the exact ADR-0078 + silently-inert failure enforcement exists to end. +- **The wire path forwards it.** `findData`'s aggregate branch passes + `having` through, and `EngineAggregateOptionsSchema` now declares it. +- The FLS predicate guard already walked `having` references + (`predicate-guard.ts`), which is what made enforcement safe to turn on. + +No migration needed: queries that carried `having` before were silently +returning every group; they now filter as written. A caller who depended on +the clause being *ignored* (sending `having` and expecting unfiltered +results) sees the corrected behavior — that is the enforcement, not a +regression. diff --git a/content/docs/data-modeling/queries.mdx b/content/docs/data-modeling/queries.mdx index b3ce6ba09d..de18ea558a 100644 --- a/content/docs/data-modeling/queries.mdx +++ b/content/docs/data-modeling/queries.mdx @@ -237,23 +237,24 @@ sort. Internal callers reaching `engine.find()` directly are unaffected. } ``` -### Cursor-Based Pagination (Keyset) — not implemented +### Keyset Pagination — a `where` predicate on the sort key + +`query.cursor` was **removed in `@objectstack/spec` 18** (#4286): nothing on the server +ever read it, so a cursor query silently returned the same first page every time. The +key is tombstoned and `QueryBuilder.cursor()` was removed with it. Express the keyset +directly — seek past the last row instead of offsetting: ```typescript +// Next page after `last` — every driver executes this, with canonicalised comparands { - limit: 20, - cursor: { id: 100 } // Opaque keyset cursor (e.g. last seen sort-key values) + where: { created_at: { $gt: last.created_at } }, + orderBy: [{ field: 'created_at', order: 'asc' }], + limit: 20 } ``` - -`cursor` is declared on `QuerySchema` as an opaque record (`Record`) -and `QueryBuilder.cursor()` will set it on a query, but **nothing on the server reads -it** — the query engine, the REST query dispatcher, and the SQL / in-memory / MongoDB -drivers all ignore it, so a cursor query silently returns the same first page every -time. Use `limit` + `offset` until keyset pagination is wired up. (There is no -`keyset`/`after` query property either.) - +(A first-class cursor, if ever designed, will be a response-minted opaque token — the +pattern the metadata-revision / flow-run / notification list endpoints already use.) --- @@ -376,13 +377,14 @@ silently returns `null` for them. Avoid these three on SQL- or memory-backed obj } ``` - -`having` is accepted by `QuerySchema` but is not enforced by the query engine today. -`EngineAggregateOptions` (the type `ObjectQL.aggregate()` actually takes) has no `having` -field, and both the REST `findData()` dispatcher and `ObjectQL.aggregate()` build their -driver-facing query from only `where` / `groupBy` / `aggregations` — a `having` clause is -silently dropped before it reaches any driver. No driver (SQL, in-memory, MongoDB) filters -on it either. Post-filter grouped results on the client until this is wired up. + +`having` is **enforced since #4286** (ADR-0049 enforce-or-remove, resolved to enforce): the +engine applies it itself AFTER aggregation, identically on the native-driver path and the +in-memory fallback, and the REST `findData()` aggregate branch forwards it. Its namespace is +the **aggregated row's own columns** — aggregation aliases (`order_count`) and groupBy +projections — with the ordinary FilterCondition operators and `$and`/`$or`/`$not`. An +unknown operator is rejected loudly rather than ignored. Native SQL `HAVING` pushdown can +come later behind a driver capability flag without changing these semantics. --- @@ -505,22 +507,24 @@ report/dashboard metadata. ## Distinct & Group By -### Distinct Records +### Distinct Records — removed flag, three live spellings + +The top-level `query.distinct` flag was **removed in `@objectstack/spec` 18** (#4286): +no driver's `find()` ever applied it, and its only observable effect was mis-wired — +it silently suppressed the REST list count while still returning duplicate rows (the +count is truthful again). The key is tombstoned and `QueryBuilder.distinct()` was +removed with it. What actually deduplicates: ```typescript -{ - object: 'task', - fields: ['category'], - distinct: true -} +// Unique combinations → groupBy +{ object: 'task', groupBy: ['category'] } + +// Deduplicated count → count_distinct +{ object: 'task', aggregations: [{ function: 'count_distinct', field: 'category', alias: 'categories' }] } ``` - -The top-level `distinct: true` flag is defined in `QuerySchema` but isn't applied by any -driver's `find()` (SQL, in-memory, and MongoDB all ignore it). A separate -`driver.distinct(object, field)` method exists on the SQL and in-memory drivers, but it isn't -called by `ObjectQL.find()`/`.aggregate()`, so it isn't reachable through a normal query. - +A separate `driver.distinct(object, field)` method also exists on the SQL and +in-memory drivers (driver-level; not called by `ObjectQL.find()`/`.aggregate()`). ### Group By with Having @@ -536,9 +540,9 @@ called by `ObjectQL.find()`/`.aggregate()`, so it isn't reachable through a norm } ``` - -As noted under [Aggregations](#aggregations) above, `having` is not currently enforced — -it's dropped before reaching the aggregation engine or any driver. + +As noted under [Aggregations](#aggregations) above, `having` filters the aggregated rows +engine-side — `total_spent` here is the aggregation alias it references. ### Date Bucketing in `groupBy` diff --git a/content/docs/deployment/troubleshooting.mdx b/content/docs/deployment/troubleshooting.mdx index cfee15bfc5..4ab68c7496 100644 --- a/content/docs/deployment/troubleshooting.mdx +++ b/content/docs/deployment/troubleshooting.mdx @@ -278,17 +278,22 @@ console.log(field.maxLength?.toString() ?? 'no limit'); 2. **Limit fields** — Only request fields you need (`fields: ['id', 'name']`) 3. **Use pagination** — Always set `limit` to avoid returning all records 4. **Avoid deep nesting** — Limit nested `$and`/`$or` depth -5. **Use cursor pagination** — For large datasets, cursor-based is faster than offset +5. **Use keyset pagination** — For large datasets, seeking past the last row is + faster than a deep `offset`. Express the keyset as a `where` predicate on the + sort key (the `cursor` query property was removed in `@objectstack/spec` 18, + #4286 — nothing ever read it) ```typescript -// Optimized query +// Optimized query — next page seeks past the last row instead of offsetting { object: 'activity', - fields: ['id', 'type', 'created_at'], // Only needed fields - where: { type: { $eq: 'login' } }, // On indexed field + fields: ['id', 'type', 'created_at'], // Only needed fields + where: { + type: { $eq: 'login' }, // On indexed field + created_at: { $lt: lastSeenCreatedAt }, // Keyset: past the last row + }, orderBy: [{ field: 'created_at', order: 'desc' }], limit: 25, - cursor: { id: lastSeenId } // Keyset/cursor pagination (field → last-seen value) } ``` diff --git a/content/docs/protocol/objectql/query-syntax.mdx b/content/docs/protocol/objectql/query-syntax.mdx index 55deed504a..e59ae73354 100644 --- a/content/docs/protocol/objectql/query-syntax.mdx +++ b/content/docs/protocol/objectql/query-syntax.mdx @@ -69,11 +69,9 @@ interface QueryAST { limit?: number; // Max records (LIMIT) offset?: number; // Skip records (OFFSET) top?: number; // Alias for limit (OData compat) - cursor?: Record; // Keyset pagination cursor aggregations?: AggregationNode[]; // Aggregation functions groupBy?: GroupByNode[]; // GROUP BY targets (string | object) - having?: FilterCondition; // HAVING clause - distinct?: boolean; // SELECT DISTINCT + having?: FilterCondition; // HAVING — engine-enforced after aggregation expand?: Record; // Recursive relation loading } ``` @@ -90,22 +88,21 @@ on the `find()` path: | Member | Status | |:-------|:-------| -| `having` | Never read: `engine.aggregate()` forwards only `object`/`where`/`groupBy`/`aggregations`, and the in-memory fallback has no HAVING stage | -| `cursor` | Accepted by `EngineQueryOptions`, but no driver implements keyset pagination | -| `distinct` | Not applied by `find()`; the SQL and in-memory drivers expose a separate `distinct(object, field, filters?)` method instead | | `aggregations[].filter` | `[EXPERIMENTAL — not enforced]` — a SQL `FILTER (WHERE …)` affordance neither the SQL builders nor the in-memory fallback applies | | `search.fuzzy` / `boost` / `operator` / `minScore` / `language` / `highlight` | `[EXPERIMENTAL — not enforced]` — only `query` and `fields` drive the expansion | `top` is the exception that *is* honored — the engine normalises it to `limit`. -Two members left this table by being **removed** (#4286, ADR-0049 enforce-or-remove): -`joins` and `windowFunctions` are tombstoned in `@objectstack/spec` 18 — a query -carrying either fails to parse with the upgrade prescription, and authoring one is a -`tsc` error. Related records are read through `expand`; window functions remain a -SQL-driver door (`SqlDriver.findWithWindowFunctions()`). The inert members that -*remain* declared (`having`, `cursor`, `distinct`, and the experimental flags above) -are tracked in the liveness ledger (`packages/spec/liveness/query.json`) pending -their #4286 dispositions. +The #4286 sweep (ADR-0049 enforce-or-remove) settled every other declared-but-inert +member. **Removed** — tombstoned in `@objectstack/spec` 18, so a query carrying one +fails to parse with the upgrade prescription and authoring it is a `tsc` error: +`joins` (related records are read through `expand`), `windowFunctions` (a SQL-driver +door remains: `SqlDriver.findWithWindowFunctions()`), `cursor` (express the keyset as +a `where` predicate on the sort key — §7), and `distinct` (unique values via +`groupBy` / `count_distinct` / the drivers' `distinct()` door; its only observable +effect was suppressing the REST list count, which is truthful again). **Enforced**: +`having` (§5). The experimental flags above are tracked in the liveness ledger +(`packages/spec/liveness/query.json`). ### Key Types @@ -659,23 +656,30 @@ const query: QueryAST = { }; ``` -### HAVING Clause (protocol only) +### HAVING Clause -`QuerySchema` defines a `having` property, but **nothing executes it**: -`engine.aggregate()` forwards only `object` / `where` / `groupBy` / `aggregations` to -the driver, no driver reads `query.having`, and the in-memory aggregation fallback has -no HAVING stage. Filter the aggregated rows in application code: +**Enforced since #4286** (ADR-0049, resolved to enforce). The engine applies `having` +itself, AFTER aggregation, identically on the native-driver path and the in-memory +fallback (`packages/objectql/src/having-filter.ts`) — the same correct-first / +optimize-later two-tier shape date bucketing uses; native SQL `HAVING` pushdown can come +later behind a driver capability flag without changing these semantics. The REST +`findData()` aggregate branch forwards the clause. + +`having` references the **aggregated row's own columns** — aggregation aliases and +groupBy projections — with the ordinary FilterCondition operators and +`$and` / `$or` / `$not`. An unknown operator rejects the query loudly rather than being +ignored (an ignored operator would silently return unfiltered aggregates — the ADR-0078 +failure mode enforcement exists to end). ```typescript +// Only accounts with > $1M pipeline const rows = await engine.aggregate('opportunity', { groupBy: ['account_id'], aggregations: [ { function: 'sum', field: 'amount', alias: 'total' }, ], + having: { total: { $gt: 1_000_000 } }, }); - -// Only accounts with > $1M pipeline -const bigAccounts = rows.filter((r) => r.total > 1_000_000); ``` ### Date Bucketing @@ -705,8 +709,14 @@ the driver's raw rows. ### Distinct -The `distinct` flag on `QueryAST` is **not applied by `find()`**. Distinct values come -from the driver's own `distinct()` method (implemented by the SQL and in-memory drivers; +`query.distinct` was **removed in `@objectstack/spec` 18** (#4286): no driver ever +rendered `SELECT DISTINCT`, and the flag's only observable effect was mis-wired — it +silently suppressed the REST list count (`total`/`hasMore` degraded to a page-local +estimate) while still returning duplicate rows. The key is tombstoned and +`QueryBuilder.distinct()` was removed with it; the count suppression is gone, so +`total` is truthful again. Unique *combinations* come from `groupBy`, deduplicated +counts from the `count_distinct` aggregation, and one column's distinct values from +the driver's own `distinct()` method (implemented by the SQL and in-memory drivers; it is not part of the `IDataDriver` contract): ```typescript @@ -823,9 +833,13 @@ const page2 = await engine.find('customer', { ### Keyset Pagination -`cursor` is accepted by `QuerySchema` and `EngineQueryOptions`, but **no driver -implements keyset pagination** — passing it has no effect. Express the keyset yourself -as an ordinary `where` predicate on the sort key: +`query.cursor` was **removed in `@objectstack/spec` 18** (#4286): no driver ever +implemented keyset pagination, so a cursor was accepted and ignored and every page came +back identical — a caller looping "until `hasMore` is false" never terminated. The key +is tombstoned (on `EngineQueryOptions` too) and `QueryBuilder.cursor()` was removed +with it. Express the keyset as an ordinary `where` predicate on the sort key — the +pattern below is the supported one; a first-class cursor, if ever designed, will be a +response-minted opaque token: ```typescript diff --git a/content/docs/references/api/contract.mdx b/content/docs/references/api/contract.mdx index 0a5ea567e6..7994f13e54 100644 --- a/content/docs/references/api/contract.mdx +++ b/content/docs/references/api/contract.mdx @@ -154,13 +154,13 @@ const result = ApiError.parse(data); | **limit** | `number` | optional | Max records to return (LIMIT) | | **offset** | `number` | optional | Records to skip (OFFSET) | | **top** | `number` | optional | Alias for limit (OData compatibility) | -| **cursor** | `Record` | optional | Cursor for keyset pagination | +| **cursor** | `any` | optional | [REMOVED] `query.cursor` was removed in @objectstack/spec 18 (#4286, ADR-0049) — no driver ever implemented keyset pagination, so the cursor was accepted and ignored and every page came back identical (a caller looping "until hasMore is false" never terminates). Delete the key; `QueryBuilder.cursor()` was removed with it. Express the keyset as an ordinary `where` predicate on your sort key — `where: { created_at: { $gt: last.created_at } }` with the matching `orderBy` — which every driver executes with canonicalised comparands. A first-class cursor, if ever built, will be a response-minted opaque token, not this caller-built record. | | **joins** | `any` | optional | [REMOVED] `query.joins` was removed in @objectstack/spec 18 (#4286, ADR-0049) — no engine or driver ever read it: a query carrying `joins` behaved exactly as if the key were absent, while its name squatted on the reserved REST parameter set. Delete the key. Related records are read through `expand` — `expand: { owner: { object: 'user', fields: ['name'] } }` — which the engine resolves via batch $in queries, and a single related column is a dotted `fields` path (`fields: ['owner.name']`). | | **aggregations** | `{ function: Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max' \| 'count_distinct' \| 'array_agg' \| 'string_agg'>; field?: string; alias: string; distinct?: boolean; … }[]` | optional | Aggregation functions | | **groupBy** | `string \| { field: string; dateGranularity?: Enum<'day' \| 'week' \| 'month' \| 'quarter' \| 'year'>; alias?: string }[]` | optional | GROUP BY targets (strings or `{field, dateGranularity?}` objects for date bucketing) | -| **having** | `any` | optional | HAVING clause for aggregation filtering | +| **having** | `any` | optional | HAVING — filter over the AGGREGATED rows (aggregation aliases + groupBy projections); applied engine-side after aggregation | | **windowFunctions** | `any` | optional | [REMOVED] `query.windowFunctions` was removed in @objectstack/spec 18 (#4286, ADR-0049) — `find()` never applied it: no engine or driver read the key on the query path, so every OVER clause it declared was silently dropped. Delete the key. Window functions are a SQL-driver capability behind `SqlDriver.findWithWindowFunctions(object, query)` (embedder-level; not on the `IDataDriver` contract or the REST surface); request-level analytics are `aggregations` + `groupBy`. | -| **distinct** | `boolean` | optional | SELECT DISTINCT flag | +| **distinct** | `any` | optional | [REMOVED] `query.distinct` was removed in @objectstack/spec 18 (#4286, ADR-0049 / ADR-0078) — no driver ever rendered SELECT DISTINCT; the flag's only observable effect was MIS-WIRED: the REST list path treated a distinct query as not countable and silently degraded `total`/`hasMore` to a page-local estimate while still returning duplicate rows. Delete the key; `QueryBuilder.distinct()` was removed with it, and the count suppression is gone (`total` is truthful again). For unique values of one column use the SQL/memory drivers' `distinct(object, field)` door; for unique combinations, `groupBy`; for a deduplicated count, the `count_distinct` aggregation. | | **expand** | `Record` | optional | Recursive relation loading map. Keys are lookup/master_detail field names; values are nested QueryAST objects that control select (`fields`) and filter (`where`, AND-merged with the batch $in), plus further expansion on the related object. The engine resolves expand via batch $in queries (driver-agnostic) with a default max depth of 3; per-parent `limit`/`offset`/`orderBy` are NOT applied on this path. | diff --git a/content/docs/references/api/protocol.mdx b/content/docs/references/api/protocol.mdx index c1e636afa9..a4ac8433b6 100644 --- a/content/docs/references/api/protocol.mdx +++ b/content/docs/references/api/protocol.mdx @@ -988,7 +988,7 @@ const result = AiAgentCapabilities.parse(data); | **skip** | `number` | optional | Records to skip (offset). | | **expand** | `string` | optional | Comma-separated list of lookup/master_detail field names to expand. Resolved to populate array and passed to the engine for batch $in expansion. | | **search** | `string` | optional | Full-text search query. | -| **distinct** | `boolean` | optional | SELECT DISTINCT flag. | +| **distinct** | `any` | optional | [REMOVED] `query.distinct` was removed in @objectstack/spec 18 (#4286, ADR-0049 / ADR-0078) — no driver ever rendered SELECT DISTINCT; the flag's only observable effect was MIS-WIRED: the REST list path treated a distinct query as not countable and silently degraded `total`/`hasMore` to a page-local estimate while still returning duplicate rows. Delete the key; `QueryBuilder.distinct()` was removed with it, and the count suppression is gone (`total` is truthful again). For unique values of one column use the SQL/memory drivers' `distinct(object, field)` door; for unique combinations, `groupBy`; for a deduplicated count, the `count_distinct` aggregation. | | **count** | `boolean` | optional | Include total count in response. | diff --git a/content/docs/references/data/data-engine.mdx b/content/docs/references/data/data-engine.mdx index 06c63802ef..9d385c34b5 100644 --- a/content/docs/references/data/data-engine.mdx +++ b/content/docs/references/data/data-engine.mdx @@ -496,6 +496,7 @@ QueryAST-aligned options for DataEngine.aggregate operations | **where** | `Record \| any` | optional | | | **groupBy** | `string[]` | optional | | | **aggregations** | `{ function: Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max' \| 'count_distinct' \| 'array_agg' \| 'string_agg'>; field?: string; alias: string; distinct?: boolean; … }[]` | optional | | +| **having** | `any` | optional | HAVING — filter over the aggregated rows (aggregation aliases + groupBy projections); applied engine-side after aggregation | | **timezone** | `string` | optional | | @@ -545,10 +546,10 @@ QueryAST-aligned query options for IDataEngine.find() operations | **limit** | `number` | optional | | | **offset** | `number` | optional | | | **top** | `number` | optional | | -| **cursor** | `Record` | optional | | +| **cursor** | `any` | optional | [REMOVED] `query.cursor` was removed in @objectstack/spec 18 (#4286, ADR-0049) — no driver ever implemented keyset pagination, so the cursor was accepted and ignored and every page came back identical (a caller looping "until hasMore is false" never terminates). Delete the key; `QueryBuilder.cursor()` was removed with it. Express the keyset as an ordinary `where` predicate on your sort key — `where: { created_at: { $gt: last.created_at } }` with the matching `orderBy` — which every driver executes with canonicalised comparands. A first-class cursor, if ever built, will be a response-minted opaque token, not this caller-built record. | | **search** | `{ query: string; fields?: string[]; fuzzy: boolean; operator: Enum<'and' \| 'or'>; … }` | optional | | | **expand** | `Record` | optional | | -| **distinct** | `boolean` | optional | | +| **distinct** | `any` | optional | [REMOVED] `query.distinct` was removed in @objectstack/spec 18 (#4286, ADR-0049 / ADR-0078) — no driver ever rendered SELECT DISTINCT; the flag's only observable effect was MIS-WIRED: the REST list path treated a distinct query as not countable and silently degraded `total`/`hasMore` to a page-local estimate while still returning duplicate rows. Delete the key; `QueryBuilder.distinct()` was removed with it, and the count suppression is gone (`total` is truthful again). For unique values of one column use the SQL/memory drivers' `distinct(object, field)` door; for unique combinations, `groupBy`; for a deduplicated count, the `count_distinct` aggregation. | --- diff --git a/content/docs/references/data/query.mdx b/content/docs/references/data/query.mdx index 633a9138b4..ea085834b8 100644 --- a/content/docs/references/data/query.mdx +++ b/content/docs/references/data/query.mdx @@ -131,13 +131,13 @@ Type: `string` | **limit** | `number` | optional | Max records to return (LIMIT) | | **offset** | `number` | optional | Records to skip (OFFSET) | | **top** | `number` | optional | Alias for limit (OData compatibility) | -| **cursor** | `Record` | optional | Cursor for keyset pagination | +| **cursor** | `any` | optional | [REMOVED] `query.cursor` was removed in @objectstack/spec 18 (#4286, ADR-0049) — no driver ever implemented keyset pagination, so the cursor was accepted and ignored and every page came back identical (a caller looping "until hasMore is false" never terminates). Delete the key; `QueryBuilder.cursor()` was removed with it. Express the keyset as an ordinary `where` predicate on your sort key — `where: { created_at: { $gt: last.created_at } }` with the matching `orderBy` — which every driver executes with canonicalised comparands. A first-class cursor, if ever built, will be a response-minted opaque token, not this caller-built record. | | **joins** | `any` | optional | [REMOVED] `query.joins` was removed in @objectstack/spec 18 (#4286, ADR-0049) — no engine or driver ever read it: a query carrying `joins` behaved exactly as if the key were absent, while its name squatted on the reserved REST parameter set. Delete the key. Related records are read through `expand` — `expand: { owner: { object: 'user', fields: ['name'] } }` — which the engine resolves via batch $in queries, and a single related column is a dotted `fields` path (`fields: ['owner.name']`). | | **aggregations** | `{ function: Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max' \| 'count_distinct' \| 'array_agg' \| 'string_agg'>; field?: string; alias: string; distinct?: boolean; … }[]` | optional | Aggregation functions | | **groupBy** | `string \| { field: string; dateGranularity?: Enum<'day' \| 'week' \| 'month' \| 'quarter' \| 'year'>; alias?: string }[]` | optional | GROUP BY targets (strings or `{field, dateGranularity?}` objects for date bucketing) | -| **having** | `any` | optional | HAVING clause for aggregation filtering | +| **having** | `any` | optional | HAVING — filter over the AGGREGATED rows (aggregation aliases + groupBy projections); applied engine-side after aggregation | | **windowFunctions** | `any` | optional | [REMOVED] `query.windowFunctions` was removed in @objectstack/spec 18 (#4286, ADR-0049) — `find()` never applied it: no engine or driver read the key on the query path, so every OVER clause it declared was silently dropped. Delete the key. Window functions are a SQL-driver capability behind `SqlDriver.findWithWindowFunctions(object, query)` (embedder-level; not on the `IDataDriver` contract or the REST surface); request-level analytics are `aggregations` + `groupBy`. | -| **distinct** | `boolean` | optional | SELECT DISTINCT flag | +| **distinct** | `any` | optional | [REMOVED] `query.distinct` was removed in @objectstack/spec 18 (#4286, ADR-0049 / ADR-0078) — no driver ever rendered SELECT DISTINCT; the flag's only observable effect was MIS-WIRED: the REST list path treated a distinct query as not countable and silently degraded `total`/`hasMore` to a page-local estimate while still returning duplicate rows. Delete the key; `QueryBuilder.distinct()` was removed with it, and the count suppression is gone (`total` is truthful again). For unique values of one column use the SQL/memory drivers' `distinct(object, field)` door; for unique combinations, `groupBy`; for a deduplicated count, the `count_distinct` aggregation. | | **expand** | `Record` | optional | Recursive relation loading map. Keys are lookup/master_detail field names; values are nested QueryAST objects that control select (`fields`) and filter (`where`, AND-merged with the batch $in), plus further expansion on the related object. The engine resolves expand via batch $in queries (driver-agnostic) with a default max depth of 3; per-parent `limit`/`offset`/`orderBy` are NOT applied on this path. | diff --git a/packages/client/src/client.test.ts b/packages/client/src/client.test.ts index 397bd576eb..e00e806f86 100644 --- a/packages/client/src/client.test.ts +++ b/packages/client/src/client.test.ts @@ -814,22 +814,13 @@ describe('QueryBuilder enhancements', () => { }); }); - it('should set cursor for keyset pagination', () => { - const q = createQuery('customer') - .cursor({ id: 'last-seen-id', created_at: '2024-01-01' }) - .build(); - expect((q as any).cursor).toEqual({ - id: 'last-seen-id', - created_at: '2024-01-01' - }); - }); - - it('should enable distinct', () => { - const q = createQuery('customer') - .select('status') - .distinct() - .build(); - expect((q as any).distinct).toBe(true); + it('cursor()/distinct() are gone with query.cursor/query.distinct (#4286)', () => { + // Both methods minted keys no executor ever read — cursor re-served + // page 1 forever; distinct only degraded the REST count. The spec + // tombstones reject the keys; the builder no longer produces them. + const q: any = createQuery('customer'); + expect(q.cursor).toBeUndefined(); + expect(q.distinct).toBeUndefined(); }); }); diff --git a/packages/client/src/query-builder.ts b/packages/client/src/query-builder.ts index 58a6a145fb..83bda8d009 100644 --- a/packages/client/src/query-builder.ts +++ b/packages/client/src/query-builder.ts @@ -288,21 +288,13 @@ export class QueryBuilder { return this; } - /** - * Set cursor for keyset pagination - */ - cursor(cursor: Record): this { - (this.query as any).cursor = cursor; - return this; - } - - /** - * Enable SELECT DISTINCT - */ - distinct(): this { - (this.query as any).distinct = true; - return this; - } + // `cursor()` and `distinct()` were REMOVED with `query.cursor` / + // `query.distinct` (#4286, spec 18): no driver ever implemented keyset + // pagination or SELECT DISTINCT, so both minted keys the engine ignored — + // `cursor` re-served page 1 forever, `distinct`'s only effect was silently + // degrading the REST count. Express a keyset as a `where` predicate on the + // sort key; deduplicate with `groupBy` / `count_distinct` or the drivers' + // `distinct(object, field)` door. /** * Build the final query AST diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index e60a44b08f..2f1ed19c0e 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -3791,6 +3791,10 @@ export class ObjectStackProtocolImplementation implements where: options.where, groupBy: options.groupBy, aggregations: options.aggregations, + // Enforced engine-side since #4286 (step 3) — dropping it here + // was finding 1: the one wire path to aggregate() lost the + // clause before any executor could ever see it. + having: options.having, context: options.context, } as any); // Apply limit client-side (EngineAggregateOptions doesn't carry limit). @@ -3823,7 +3827,11 @@ export class ObjectStackProtocolImplementation implements let total = records.length; let hasMore = false; if (pageLimit !== undefined) { - const countable = options.search == null && options.distinct == null; + // `distinct` used to suppress the count here too — #4286 finding 2: + // the flag's ONLY observable effect platform-wide, on a capability + // that never deduplicated a row. Removed with `query.distinct` + // (tombstoned in spec 18); `total`/`hasMore` are truthful again. + const countable = options.search == null; if (countable) { try { total = await this.engine.count(request.object, { diff --git a/packages/objectql/src/engine-aggregate-having.test.ts b/packages/objectql/src/engine-aggregate-having.test.ts new file mode 100644 index 0000000000..a80d0938dd --- /dev/null +++ b/packages/objectql/src/engine-aggregate-having.test.ts @@ -0,0 +1,151 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// `engine.aggregate({ having })` — ENFORCED since #4286 (step 3). +// +// `having` had been declared on the request surface since AST v2 and executed +// by nothing (finding 1: aggregate() rebuilt the driver AST without it, so +// even a driver implementing HAVING could never receive it). It is now +// engine-owned: applied AFTER aggregation over the aggregated row's own +// columns, identically on the native-driver path and the in-memory fallback. +// These tests pin both paths and the identical-semantics seam between them. + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; + +const ROWS = [ + { customer_id: 'c1', amount: 100 }, + { customer_id: 'c1', amount: 400 }, + { customer_id: 'c2', amount: 900 }, + { customer_id: 'c2', amount: 300 }, + { customer_id: 'c2', amount: 50 }, + { customer_id: 'c3', amount: 20 }, +]; + +/** A driver WITH native aggregate() (groups + sums itself, no HAVING). */ +function makeNativeDriver(rows: any[]) { + let nativeAggregateCalls = 0; + const driver: any = { + name: 'native-agg-mock', + version: '0.0.0', + supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find() { return rows.slice(); }, + findStream() { throw new Error('ni'); }, + async findOne() { return rows[0] ?? null; }, + async create(_o: string, d: any) { return d; }, + async update(_o: string, _id: string, d: any) { return d; }, + async delete() { return true; }, + async count() { return rows.length; }, + async bulkCreate(_o: string, r: any[]) { return r; }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + async aggregate(_object: string, ast: any) { + nativeAggregateCalls += 1; + const groups = new Map(); + for (const r of rows) { + const g = groups.get(r.customer_id) ?? { customer_id: r.customer_id, order_count: 0, total: 0 }; + g.order_count += 1; + g.total += r.amount; + groups.set(r.customer_id, g); + } + // A real native driver ignores ast.having today — the engine post-filter + // is what makes the clause live. Return the FULL grouped set. + void ast; + return Array.from(groups.values()); + }, + }; + return { driver, nativeCalls: () => nativeAggregateCalls }; +} + +/** A driver WITHOUT aggregate() — engine falls back to find() + in-memory. */ +function makeRawDriver(rows: any[]) { + const driver: any = { + name: 'raw-mock', + version: '0.0.0', + supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find() { return rows.slice(); }, + findStream() { throw new Error('ni'); }, + async findOne() { return rows[0] ?? null; }, + async create(_o: string, d: any) { return d; }, + async update(_o: string, _id: string, d: any) { return d; }, + async delete() { return true; }, + async count() { return rows.length; }, + async bulkCreate(_o: string, r: any[]) { return r; }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return driver; +} + +async function makeEngine(driver: any) { + const engine = new ObjectQL(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject({ + name: 'order', + fields: { customer_id: { type: 'text' }, amount: { type: 'number' } }, + } as any); + return engine; +} + +const AGG_QUERY = { + groupBy: ['customer_id'], + aggregations: [ + { function: 'count', alias: 'order_count' }, + { function: 'sum', field: 'amount', alias: 'total' }, + ], +}; + +describe('engine.aggregate — having (#4286 step 3)', () => { + it('applies having AFTER the native driver aggregate', async () => { + const { driver, nativeCalls } = makeNativeDriver(ROWS); + const engine = await makeEngine(driver); + + const rows = await engine.aggregate('order', { + ...AGG_QUERY, + having: { order_count: { $gt: 1 } }, + } as any); + + expect(nativeCalls()).toBe(1); // native path taken… + expect(rows.map((r: any) => r.customer_id).sort()).toEqual(['c1', 'c2']); // …and post-filtered + }); + + it('applies having on the in-memory fallback path with identical semantics', async () => { + const engine = await makeEngine(makeRawDriver(ROWS)); + + const rows = await engine.aggregate('order', { + ...AGG_QUERY, + having: { order_count: { $gt: 1 } }, + } as any); + + expect(rows.map((r: any) => r.customer_id).sort()).toEqual(['c1', 'c2']); + }); + + it('having references the aggregation ALIAS namespace, composing with $and', async () => { + const engine = await makeEngine(makeRawDriver(ROWS)); + + const rows = await engine.aggregate('order', { + ...AGG_QUERY, + having: { $and: [{ order_count: { $gte: 2 } }, { total: { $gt: 600 } }] }, + } as any); + + expect(rows).toEqual([{ customer_id: 'c2', order_count: 3, total: 1250 }]); + }); + + it('an absent having returns the full grouped set (no behavior change)', async () => { + const engine = await makeEngine(makeRawDriver(ROWS)); + const rows = await engine.aggregate('order', { ...AGG_QUERY } as any); + expect(rows).toHaveLength(3); + }); + + it('an unknown having operator REJECTS the aggregate instead of ignoring it', async () => { + const engine = await makeEngine(makeRawDriver(ROWS)); + await expect(engine.aggregate('order', { + ...AGG_QUERY, + having: { order_count: { $median: 2 } }, + } as any)).rejects.toThrow(/Unsupported operator '\$median' in `having`/); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index e8880c780d..df867f2174 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -72,6 +72,7 @@ import { bindHooksToEngine } from './hook-binder.js'; import { validateRecord, normalizeMultiValueFields, coerceBooleanFields, ValidationError, valueShapePostureSetByEnv, mediaPostureSetByEnv, isScannableValueShapeField } from './validation/record-validator.js'; import { evaluateValidationRules, needsPriorRecord, stripReadonlyWhenFields, stripReadonlyWhenFieldsMulti, hasReadonlyWhenInPayload, stripReadonlyFields } from './validation/rule-validator.js'; import { applyInMemoryAggregation } from './in-memory-aggregation.js'; +import { applyHaving } from './having-filter.js'; /** * The lifecycle events the engine actually dispatches via `triggerHooks`. This @@ -3983,6 +3984,11 @@ export class ObjectQL implements IDataEngine { where: query.where, groupBy: query.groupBy as any, aggregations: query.aggregations, + // ENFORCED since #4286 (step 3). On the ast so the FLS predicate + // guard walks its references (predicate-guard.ts) and a future + // native pushdown has it in hand; the engine's post-aggregation + // applyHaving() below is authoritative either way. + having: query.having, }, options: query, context: mergeReadContext(query?.context, options?.context), @@ -4022,7 +4028,14 @@ export class ObjectQL implements IDataEngine { const hasDateBucket = structuredItems.some((g: any) => !!g?.dateGranularity); const tzRequiresInMemory = !!tz && tz !== 'UTC' && hasDateBucket; if (typeof drv.aggregate === 'function' && allStructuredSupported && !tzRequiresInMemory) { - return drv.aggregate(object, ast, this.buildDriverOptions(object, opCtx.context)); + // HAVING is engine-owned (#4286): applied AFTER aggregation, over + // the aggregated row's own columns (aggregation aliases + groupBy + // projections), identically on both paths. No driver implements it + // natively today; one that grows native support must advertise a + // capability flag, at which point this post-filter becomes the + // fallback tier — the dateGranularity two-tier pattern. + const aggregated = await drv.aggregate(object, ast, this.buildDriverOptions(object, opCtx.context)); + return applyHaving(aggregated, ast.having); } // In-memory fallback path: ask the driver for raw rows, then bucket + // aggregate here. This guarantees `groupBy` (incl. structured items @@ -4031,7 +4044,7 @@ export class ObjectQL implements IDataEngine { // driver-memory, partial SQL drivers), and is the path that honours a // non-UTC reference timezone. const raw = await driver.find(object, ast, this.buildDriverOptions(object, opCtx.context)); - return applyInMemoryAggregation(raw, ast, tz); + return applyHaving(applyInMemoryAggregation(raw, ast, tz), ast.having); }); return opCtx.result as any[]; diff --git a/packages/objectql/src/having-filter.test.ts b/packages/objectql/src/having-filter.test.ts new file mode 100644 index 0000000000..4e81cdd699 --- /dev/null +++ b/packages/objectql/src/having-filter.test.ts @@ -0,0 +1,80 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * HAVING evaluator (#4286 step 3) — semantics over AGGREGATED rows. + * + * The namespace is the aggregated row's own columns (aggregation aliases + + * groupBy projections); operator semantics mirror the Filter Protocol's + * memory evaluation, EXCEPT that an unknown operator throws — ignoring one + * would silently return unfiltered aggregates, the exact silently-inert + * failure (#4286, ADR-0078) enforcement exists to end. + */ + +import { describe, it, expect } from 'vitest'; +import { applyHaving, matchesHaving } from './having-filter.js'; + +const ROWS = [ + { customer_id: 'c1', order_count: 2, total: 500 }, + { customer_id: 'c2', order_count: 6, total: 1200 }, + { customer_id: 'c3', order_count: 9, total: 800, region: null }, +]; + +describe('applyHaving', () => { + it('returns rows unchanged for an absent or empty condition', () => { + expect(applyHaving(ROWS, undefined)).toBe(ROWS); + expect(applyHaving(ROWS, null)).toBe(ROWS); + expect(applyHaving(ROWS, {})).toBe(ROWS); + }); + + it('filters on an aggregation alias with a comparison operator', () => { + expect(applyHaving(ROWS, { order_count: { $gt: 5 } }).map((r) => r.customer_id)) + .toEqual(['c2', 'c3']); + expect(applyHaving(ROWS, { total: { $lte: 800 } }).map((r) => r.customer_id)) + .toEqual(['c1', 'c3']); + }); + + it('filters on a groupBy projection with implicit equality', () => { + expect(applyHaving(ROWS, { customer_id: 'c2' })).toHaveLength(1); + }); + + it('composes with $and / $or / $not', () => { + expect(applyHaving(ROWS, { + $and: [{ order_count: { $gt: 1 } }, { total: { $gt: 1000 } }], + }).map((r) => r.customer_id)).toEqual(['c2']); + + expect(applyHaving(ROWS, { + $or: [{ total: { $gt: 1000 } }, { order_count: { $gte: 9 } }], + }).map((r) => r.customer_id)).toEqual(['c2', 'c3']); + + expect(applyHaving(ROWS, { $not: { customer_id: 'c1' } })).toHaveLength(2); + }); + + it('supports set, range, and null operators', () => { + expect(applyHaving(ROWS, { customer_id: { $in: ['c1', 'c3'] } })).toHaveLength(2); + expect(applyHaving(ROWS, { total: { $between: [600, 1300] } }).map((r) => r.customer_id)) + .toEqual(['c2', 'c3']); + expect(applyHaving(ROWS, { region: { $null: true } }).map((r) => r.customer_id)) + .toEqual(['c1', 'c2', 'c3']); // absent folds into null, like the memory matcher + }); + + it('multiple keys on one condition AND together, like `where`', () => { + expect(applyHaving(ROWS, { order_count: { $gte: 2 }, total: { $lt: 900 } }) + .map((r) => r.customer_id)).toEqual(['c1', 'c3']); + }); +}); + +describe('matchesHaving — the unknown-operator refusal', () => { + it('THROWS on an unknown condition operator instead of ignoring it', () => { + expect(() => matchesHaving(ROWS[0], { order_count: { $median: 3 } })) + .toThrow(/Unsupported operator '\$median' in `having`.*\$gte.*unfiltered aggregates/s); + }); + + it('THROWS on an unknown logical operator', () => { + expect(() => matchesHaving(ROWS[0], { $nand: [{ order_count: 2 }] })) + .toThrow(/Unsupported operator '\$nand'/); + }); + + it('accepts $regex with $options as its sibling, not an operator', () => { + expect(matchesHaving({ k: 'Alpha' }, { k: { $regex: '^alp', $options: 'i' } })).toBe(true); + }); +}); diff --git a/packages/objectql/src/having-filter.ts b/packages/objectql/src/having-filter.ts new file mode 100644 index 0000000000..11da0ca228 --- /dev/null +++ b/packages/objectql/src/having-filter.ts @@ -0,0 +1,155 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// HAVING — the engine-side filter over AGGREGATED rows (#4286 step 3). +// +// `query.having` was declared on the request surface since AST v2 and executed +// by nothing: `engine.aggregate()` rebuilt the driver AST without it (so a +// driver implementing HAVING would never have received it), and the in-memory +// fallback had no HAVING stage — an ADR-0078 silently-inert declaration on the +// one clause every SQL-literate author (human or model) expects to work next +// to `groupBy` / `aggregations`. ADR-0049 resolved it to ENFORCE: the engine +// applies `having` itself, uniformly, AFTER aggregation — the same +// correct-first / optimize-later two-tier shape date bucketing uses. Native +// SQL pushdown can come later behind a driver capability flag without changing +// the semantics defined here. +// +// SEMANTICS. `having` filters the AGGREGATED result rows, so the namespace it +// references is the aggregated row's own columns: aggregation aliases +// (`order_count`, `total`) and groupBy projections (the field name, or the +// item's `alias` for structured entries — after date bucketing). It is an +// ordinary FilterCondition over those columns: implicit equality, the +// comparison / set / null / existence / string operators, and `$and` / `$or` / +// `$not` composition. Operator semantics mirror the Filter Protocol as +// driver-memory's matcher implements it, with ONE deliberate divergence: +// +// AN UNKNOWN OPERATOR THROWS. The memory matcher ignores operators it does not +// know; here an ignored operator would silently return UNFILTERED aggregates — +// the precise failure mode (#4286, ADR-0078) this module exists to end. The +// rejection names the operator and the supported set. + +import type { FilterCondition } from '@objectstack/spec/data'; + +const LOGICAL_OPERATORS = ['$and', '$or', '$not'] as const; + +const CONDITION_OPERATORS = [ + '$eq', '$ne', '$gt', '$gte', '$lt', '$lte', '$between', + '$in', '$nin', '$exists', '$null', + '$contains', '$notContains', '$startsWith', '$endsWith', '$regex', +] as const; + +function unknownOperator(op: string, where: 'logical' | 'condition'): Error { + const supported = where === 'logical' + ? `${LOGICAL_OPERATORS.join(', ')} (or a column condition)` + : CONDITION_OPERATORS.join(', '); + return new Error( + `Unsupported operator '${op}' in \`having\`. HAVING filters the aggregated rows ` + + `(aggregation aliases + groupBy projections) and supports: ${supported}. ` + + `An unknown operator is refused rather than ignored — ignoring it would silently ` + + `return unfiltered aggregates (#4286, ADR-0078).`, + ); +} + +/** + * Filter aggregated rows by the query's `having` condition. An absent or empty + * condition returns the rows unchanged (same vacuous-filter convention as + * `where`). + */ +export function applyHaving(rows: any[], having: FilterCondition | null | undefined): any[] { + if (!having || typeof having !== 'object' || Object.keys(having).length === 0) return rows; + return rows.filter((row) => matchesHaving(row, having)); +} + +/** Evaluate one aggregated row against a HAVING FilterCondition. */ +export function matchesHaving(row: Record, cond: any): boolean { + if (!cond || typeof cond !== 'object') return true; + for (const [key, value] of Object.entries(cond)) { + if (key === '$and') { + const branches = Array.isArray(value) ? value : [value]; + if (!branches.every((c) => matchesHaving(row, c))) return false; + continue; + } + if (key === '$or') { + const branches = Array.isArray(value) ? value : [value]; + if (!branches.some((c) => matchesHaving(row, c))) return false; + continue; + } + if (key === '$not') { + if (matchesHaving(row, value)) return false; + continue; + } + if (key.startsWith('$')) throw unknownOperator(key, 'logical'); + // Aggregated rows are flat (aliases + group projections) — direct access, + // no dotted-path resolution. + if (!checkCondition(row?.[key], value)) return false; + } + return true; +} + +/** One column's condition — implicit equality or an operator object. */ +function checkCondition(value: any, condition: any): boolean { + // Implicit equality (primitives, null, Date, array exact-match) — loose `==` + // to mirror the Filter Protocol's memory evaluation. + if ( + typeof condition !== 'object' + || condition === null + || condition instanceof Date + || Array.isArray(condition) + ) { + // eslint-disable-next-line eqeqeq + return value == condition; + } + + const keys = Object.keys(condition); + const isOperatorObject = keys.some((k) => k.startsWith('$')); + if (!isOperatorObject) { + // Plain-object implicit equality — structural compare, as the matcher does. + return JSON.stringify(value) === JSON.stringify(condition); + } + + for (const op of keys) { + if (op === '$options') continue; // consumed by $regex below + const target = (condition as Record)[op]; + if (value === undefined && op !== '$exists' && op !== '$ne' && op !== '$null') return false; + switch (op) { + // eslint-disable-next-line eqeqeq + case '$eq': if (value != target) return false; break; + // eslint-disable-next-line eqeqeq + case '$ne': if (value == target) return false; break; + case '$gt': if (!(value > target)) return false; break; + case '$gte': if (!(value >= target)) return false; break; + case '$lt': if (!(value < target)) return false; break; + case '$lte': if (!(value <= target)) return false; break; + case '$between': + if (Array.isArray(target) && (value < target[0] || value > target[1])) return false; + break; + case '$in': if (!Array.isArray(target) || !target.includes(value)) return false; break; + case '$nin': if (Array.isArray(target) && target.includes(value)) return false; break; + case '$exists': { + const exists = value !== undefined && value !== null; + if (exists !== !!target) return false; + break; + } + case '$null': + if (target === true && value != null) return false; + if (target === false && value == null) return false; + break; + case '$contains': if (typeof value !== 'string' || !value.includes(target)) return false; break; + case '$notContains': if (typeof value !== 'string' || value.includes(target)) return false; break; + case '$startsWith': if (typeof value !== 'string' || !value.startsWith(target)) return false; break; + case '$endsWith': if (typeof value !== 'string' || !value.endsWith(target)) return false; break; + case '$regex': { + let re: RegExp; + try { + re = new RegExp(target, (condition as Record).$options || ''); + } catch { + return false; + } + if (!re.test(String(value))) return false; + break; + } + default: + throw unknownOperator(op, 'condition'); + } + } + return true; +} diff --git a/packages/objectql/src/protocol-data.test.ts b/packages/objectql/src/protocol-data.test.ts index ec7b6d255c..2ff0278ff1 100644 --- a/packages/objectql/src/protocol-data.test.ts +++ b/packages/objectql/src/protocol-data.test.ts @@ -970,6 +970,28 @@ describe('ObjectStackProtocolImplementation - Data Operations', () => { expect(engine.find.mock.calls[0][1].where).toBeUndefined(); }); + it('forwards `having` to engine.aggregate — the wire half of #4286 step 3', async () => { + // Finding 1 of #4286: this branch rebuilt the aggregate options + // without `having`, so the one wire path to aggregate() lost the + // clause before any executor could see it. Now it forwards. + const { protocol, engine } = makeProtocol(); + (engine as any).aggregate = vi.fn().mockResolvedValue([ + { status: 'done', task_count: 7 }, + ]); + await protocol.findData({ + object: 'showcase_task', + query: { + groupBy: ['status'], + aggregations: [{ function: 'count', alias: 'task_count' }], + having: { task_count: { $gt: 5 } }, + }, + }); + expect((engine as any).aggregate).toHaveBeenCalledOnce(); + expect((engine as any).aggregate.mock.calls[0][1].having) + .toEqual({ task_count: { $gt: 5 } }); + expect(engine.find).not.toHaveBeenCalled(); + }); + it('stands down when the schema carries no field map — nothing to check against', async () => { // External datasources / engine doubles whose columns are not // mirrored locally. Same tiering as the #3770 object gate one level diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 41a8e22c07..ae7a616f49 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -467,6 +467,8 @@ "PoolConfig (type)", "PoolConfigSchema (const)", "ProvisionPrimaryOptions (interface)", + "QUERY_CURSOR_REMOVED (const)", + "QUERY_DISTINCT_REMOVED (const)", "QueryAST (type)", "QueryFilter (type)", "QueryFilterSchema (const)", diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index e1b4c4759e..933b9f0740 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -1204,7 +1204,7 @@ "api/GetWorkflowStateResponse:recordId", "api/GetWorkflowStateResponse:state", "api/HttpFindQueryParams:count", - "api/HttpFindQueryParams:distinct", + "api/HttpFindQueryParams:distinct [RETIRED]", "api/HttpFindQueryParams:expand", "api/HttpFindQueryParams:filter", "api/HttpFindQueryParams:filters", @@ -3367,6 +3367,7 @@ "data/EngineAggregateOptions:aggregations", "data/EngineAggregateOptions:context", "data/EngineAggregateOptions:groupBy", + "data/EngineAggregateOptions:having", "data/EngineAggregateOptions:timezone", "data/EngineAggregateOptions:where", "data/EngineCountOptions:context", @@ -3375,8 +3376,8 @@ "data/EngineDeleteOptions:multi", "data/EngineDeleteOptions:where", "data/EngineQueryOptions:context", - "data/EngineQueryOptions:cursor", - "data/EngineQueryOptions:distinct", + "data/EngineQueryOptions:cursor [RETIRED]", + "data/EngineQueryOptions:distinct [RETIRED]", "data/EngineQueryOptions:expand", "data/EngineQueryOptions:fields", "data/EngineQueryOptions:limit", @@ -3724,8 +3725,8 @@ "data/PoolConfig:max", "data/PoolConfig:min", "data/Query:aggregations", - "data/Query:cursor", - "data/Query:distinct", + "data/Query:cursor [RETIRED]", + "data/Query:distinct [RETIRED]", "data/Query:expand", "data/Query:fields", "data/Query:groupBy", diff --git a/packages/spec/liveness/README.md b/packages/spec/liveness/README.md index 5b393bf2bc..b7ea453ac6 100644 --- a/packages/spec/liveness/README.md +++ b/packages/spec/liveness/README.md @@ -502,7 +502,7 @@ EOF | report | 13 | 0 | 0 | – | dataset-bound (ADR-0021); the aria/performance LEDGER entries were stale — the keys left the schema in the report-liveness close-out; deleted 2026-07-30 as hygiene. Audit-era `chart` DEAD superseded (framework#1890 / #3441) | | dashboard | 10 | 0 | 2 | – | ADR-0021 dataset widgets (#3251; DashboardWidgetSchema `.strict()`); `aria`/`performance` (and widget `performance` + PerformanceConfigSchema) REMOVED 2026-07-30 (#3896 close-out sweep — no renderer applied any of them); audit-era `globalFilters`/`dateRange` DEAD superseded (framework#2501) | -| query | 15 | 7 | 5 | – | **not a metadata type** — the REQUEST surface (`QuerySchema`: client SDK QueryBuilder output; the `POST /data/:object/query` body), governed via `SPEC_ONLY_SCHEMAS` (#4286). The 7 experimental resolve from `[EXPERIMENTAL — not enforced]` describe markers, not ledger entries (search `fuzzy`/`operator`/`boost`/`minScore`/`language`/`highlight` + `aggregations[].filter` — declared engine affordances no executor receives). dead = the #4286 worklist: `joins`/`windowFunctions` REMOVED 2026-07-31 (tombstoned; protocol-18 semantic migrations `query-joins-retired`/`query-window-functions-retired`; the JoinNode and WindowFunctionNode clusters deleted with them), `cursor`/`distinct` (shipped SDK producers, no executor — `distinct` is MIS-WIRED: its only effect is REST count-suppression, protocol.ts:3816) and `having` (never reaches a driver — `engine.aggregate()` rebuilds the AST without it) pending their #4286 step-3/4 dispositions | +| query | 16 | 7 | 4 | – | **not a metadata type** — the REQUEST surface (`QuerySchema`: client SDK QueryBuilder output; the `POST /data/:object/query` body), governed via `SPEC_ONLY_SCHEMAS` (#4286). The 7 experimental resolve from `[EXPERIMENTAL — not enforced]` describe markers, not ledger entries (search `fuzzy`/`operator`/`boost`/`minScore`/`language`/`highlight` + `aggregations[].filter` — declared engine affordances no executor receives). The #4286 sweep closed out same-release: `having` ENFORCED 2026-07-31 (engine-side post-aggregation filter, both paths; was finding 1); dead 4 = the tombstoned removals `joins`/`windowFunctions`/`cursor`/`distinct` — REMOVED 2026-07-31 (retiredKey keeps each in the walked shape so the rows stay; protocol-18 semantic migrations; the JoinNode + WindowFunctionNode clusters and the `QueryBuilder.cursor()`/`.distinct()` producers deleted with their keys; `distinct`'s mis-wired REST count suppression deleted too — finding 2) | | webhook | 0 | 1 | 16 | – | **not a registered metadata type** — governed via the gate's spec-only schema override (`SPEC_ONLY_SCHEMAS`), not `getMetadataTypeSchema` (#3461/#3462). The ENTIRE authoring surface is dead: nothing materializes an authored `webhooks:` entry into a `sys_webhook` dispatcher row (#3461, enforce-or-remove pending). `url` carries the single per-webhook `authorWarn` (one no-op heads-up per artifact, not per-prop); `authentication` experimental (HMAC-`secret`-only); `isActive` unmarked (default(true)). Notes cite the sys_webhook column map as the future materializer's mapping table | The `dead` set across types is the enforce-or-remove worklist (ADR-0049); every diff --git a/packages/spec/liveness/query.json b/packages/spec/liveness/query.json index 8b244f02c5..e417f36d4c 100644 --- a/packages/spec/liveness/query.json +++ b/packages/spec/liveness/query.json @@ -1,6 +1,6 @@ { "type": "query", - "_note": "QuerySchema (BaseQuerySchema + expand) — packages/spec/src/data/query.zod.ts. NOT a registered metadata type: QueryAST is the REQUEST surface (the client SDK QueryBuilder's output; the POST /data/:object/query body), governed via the gate's SPEC_ONLY_SCHEMAS override like webhook (#4286). It is authorable by every API caller yet never stored as stack metadata, which is why no other gate reached it: this ledger read the metadata-type registry, check:authorable-surface walks the same registry, and check:exported-any is a type-resolution gate. Seeded 2026-07-31 from the #4286 sweep — joins/windowFunctions REMOVED (tombstoned; protocol-18 semantic migrations), search sub-flags + aggregations.filter marked experimental at the schema, cursor/distinct/having dead pending their #4286 dispositions. The main executor evidence below: objectql engine find/aggregate and the drivers' find paths.", + "_note": "QuerySchema (BaseQuerySchema + expand) — packages/spec/src/data/query.zod.ts. NOT a registered metadata type: QueryAST is the REQUEST surface (the client SDK QueryBuilder's output; the POST /data/:object/query body), governed via the gate's SPEC_ONLY_SCHEMAS override like webhook (#4286). It is authorable by every API caller yet never stored as stack metadata, which is why no other gate reached it: this ledger read the metadata-type registry, check:authorable-surface walks the same registry, and check:exported-any is a type-resolution gate. Seeded 2026-07-31 from the #4286 sweep; closed out same day — joins/windowFunctions/cursor/distinct REMOVED (tombstoned; protocol-18 semantic migrations; the cursor/distinct SDK producers deleted with their keys), having ENFORCED engine-side, search sub-flags + aggregations.filter marked experimental at the schema. The main executor evidence below: objectql engine find/aggregate and the drivers' find paths.", "props": { "object": { "status": "live", @@ -55,7 +55,7 @@ "cursor": { "status": "dead", "verifiedAt": "2026-07-31", - "note": "#4286 Tier A: a public SDK method mints it — QueryBuilder.cursor() (packages/client/src/query-builder.ts:294), serialized to the wire at 5 sites in client/src/index.ts, and re-declared on EngineQueryOptionsSchema — but no driver implements keyset pagination, so it has no effect. Disposition PENDING (#4286 step 4): removal is ADR-0087 conversion/tombstone work that must take the SDK producer and the wire sites with it." + "note": "REMOVED 2026-07-31 (#4286 step 4, resolved to remove) — tombstoned at the schema (retiredKey; also on EngineQueryOptionsSchema, same prescription) and the shipped producer QueryBuilder.cursor() deleted with it: no driver implemented keyset pagination, so the cursor re-served page 1 forever. The protocol-18 semantic migration `query-cursor-retired` carries it (request surface, no source to rewrite). The entry stays because retiredKey keeps the key in the walked shape. Manual keyset (a `where` predicate on the sort key) is the documented pattern; a first-class cursor, if ever designed, will be a response-minted opaque token. NOTE: the `cursor: string` params on listRevisions / flow-runs / notifications in client/src/index.ts are those endpoints' own server-minted tokens — live, unrelated, untouched." }, "joins": { "status": "dead", @@ -93,9 +93,10 @@ "evidence": "packages/objectql/src/engine.ts aggregate() forwards groupBy; packages/objectql/src/in-memory-aggregation.ts buckets by it (including dateGranularity fallback)" }, "having": { - "status": "dead", + "status": "live", "verifiedAt": "2026-07-31", - "note": "Never delivered: engine.aggregate() rebuilds the AST it hands the driver with exactly object/where/groupBy/aggregations, so a driver that DID implement HAVING would never receive it (#4286 finding 1), and the in-memory fallback has no HAVING stage. The strongest ENFORCE candidate of the #4286 set — a genuinely expected SQL capability the fallback could apply in a few lines — so it is deliberately left declared pending that call (#4286 step 3). The FLS predicate guard already walks it (predicate-guard.ts:64), so enforcing it later leaks nothing." + "evidence": "packages/objectql/src/having-filter.ts (the evaluator); packages/objectql/src/engine.ts aggregate() applies it after BOTH the native-driver path and the in-memory fallback; packages/metadata-protocol/src/protocol.ts findData aggregate branch forwards it; packages/spec/src/data/data-engine.zod.ts EngineAggregateOptionsSchema declares it", + "note": "ENFORCED 2026-07-31 (#4286 step 3, resolved to enforce): engine-owned, applied AFTER aggregation over the aggregated row's namespace (aggregation aliases + groupBy projections); an unknown operator rejects loudly instead of being ignored. Was #4286 finding 1 — aggregate() rebuilt the driver AST without it, so even a HAVING-implementing driver could never receive it. Native SQL pushdown can come later behind a driver capability flag (the dateGranularity two-tier pattern) without changing semantics. The FLS predicate guard walks it (predicate-guard.ts:64)." }, "windowFunctions": { "status": "dead", @@ -105,8 +106,7 @@ "distinct": { "status": "dead", "verifiedAt": "2026-07-31", - "authorWarn": true, - "note": "#4286 finding 2 — MIS-WIRED, not merely dead (ADR-0078 treats this more harshly): no driver renders SELECT DISTINCT, but metadata-protocol/src/protocol.ts:3816 treats `options.distinct != null` as not-countable, so a distinct query silently skips engine.count() and degrades the REST total/hasMore to a page-local estimate — the caller gets non-distinct rows AND worse pagination metadata. QueryBuilder.distinct() (packages/client/src/query-builder.ts:302) mints it; also declared on EngineQueryOptionsSchema. Disposition PENDING (#4286 step 4): removal or enforcement must handle the count-suppression side effect deliberately either way." + "note": "REMOVED 2026-07-31 (#4286 step 4, resolved to remove) — tombstoned at the schema (retiredKey; also on EngineQueryOptionsSchema, same prescription), the shipped producer QueryBuilder.distinct() deleted, AND the mis-wiring deleted with it: finding 2 was that the flag's only observable effect was suppressing the REST list count (protocol.ts `countable`), so callers got duplicate rows plus degraded total/hasMore — worse than dead, a side effect that 'confirmed' a capability that never ran. `total` is truthful for those queries again (the observable REST change, called out in the changeset). The protocol-18 semantic migration `query-distinct-retired` carries it. The entry stays because retiredKey keeps the key in the walked shape. Live spellings: groupBy, count_distinct, the drivers' distinct(object, field) door. AggregationNode.distinct (per-aggregation dedupe) is a different, live member — see aggregations.children." }, "expand": { "status": "live", diff --git a/packages/spec/src/api/protocol.test.ts b/packages/spec/src/api/protocol.test.ts index 5bf6a563f0..6990d26137 100644 --- a/packages/spec/src/api/protocol.test.ts +++ b/packages/spec/src/api/protocol.test.ts @@ -557,17 +557,22 @@ describe('HttpFindQueryParamsSchema', () => { } }); - it('should accept sort, orderBy, expand, search, distinct, count', () => { + it('should accept sort, orderBy, expand, search, count', () => { const result = HttpFindQueryParamsSchema.safeParse({ sort: 'name asc,created_at desc', expand: 'owner,account', search: 'John', - distinct: true, count: true, }); expect(result.success).toBe(true); }); + it('rejects ?distinct — the querystring spelling of the removed query.distinct (#4286)', () => { + const result = HttpFindQueryParamsSchema.safeParse({ distinct: true }); + expect(result.success).toBe(false); + expect(JSON.stringify(result.error?.issues)).toMatch(/query\.distinct.*removed/s); + }); + it('should accept empty object (all params optional)', () => { expect(HttpFindQueryParamsSchema.safeParse({}).success).toBe(true); }); diff --git a/packages/spec/src/api/protocol.zod.ts b/packages/spec/src/api/protocol.zod.ts index f5b1f70b7b..33d3f5c62c 100644 --- a/packages/spec/src/api/protocol.zod.ts +++ b/packages/spec/src/api/protocol.zod.ts @@ -10,7 +10,8 @@ import { DeleteManyRequestSchema, } from './batch.zod'; import { MetadataCacheRequestSchema, MetadataCacheResponseSchema } from './http-cache.zod'; -import { QuerySchema } from '../data/query.zod'; +import { QuerySchema, QUERY_DISTINCT_REMOVED } from '../data/query.zod'; +import { retiredKey } from '../shared/retired-key'; import { DroppedFieldsEventSchema } from '../data/data-engine.zod'; import { AnalyticsQueryRequestSchema, @@ -335,7 +336,8 @@ export const HttpFindQueryParamsSchema = lazySchema(() => z.object({ + 'Resolved to populate array and passed to the engine for batch $in expansion.' ), search: z.string().optional().describe('Full-text search query.'), - distinct: z.coerce.boolean().optional().describe('SELECT DISTINCT flag.'), + /** `?distinct` — REMOVED (#4286): the querystring spelling of `query.distinct`, same tombstone. */ + distinct: retiredKey(QUERY_DISTINCT_REMOVED), count: z.coerce.boolean().optional().describe('Include total count in response.'), })); diff --git a/packages/spec/src/data/data-engine.test.ts b/packages/spec/src/data/data-engine.test.ts index 68ba436c19..063a737386 100644 --- a/packages/spec/src/data/data-engine.test.ts +++ b/packages/spec/src/data/data-engine.test.ts @@ -369,13 +369,18 @@ describe('EngineQueryOptionsSchema', () => { limit: 50, offset: 0, expand: { owner: { object: 'user' } }, - distinct: true, }); expect(options.where).toBeDefined(); expect(options.fields).toHaveLength(3); expect(options.limit).toBe(50); expect(options.expand!.owner.object).toBe('user'); - expect(options.distinct).toBe(true); + }); + + it('rejects the removed cursor/distinct keys with the query.* prescriptions (#4286)', () => { + expect(() => EngineQueryOptionsSchema.parse({ cursor: { id: 'x' } })) + .toThrow(/query\.cursor.*removed/s); + expect(() => EngineQueryOptionsSchema.parse({ distinct: true })) + .toThrow(/query\.distinct.*removed/s); }); it('should accept context', () => { diff --git a/packages/spec/src/data/data-engine.zod.ts b/packages/spec/src/data/data-engine.zod.ts index 8de6fc00f9..34cb16a371 100644 --- a/packages/spec/src/data/data-engine.zod.ts +++ b/packages/spec/src/data/data-engine.zod.ts @@ -2,7 +2,8 @@ import { z } from 'zod'; import { FilterConditionSchema } from './filter.zod'; -import { SortNodeSchema, QuerySchema, FullTextSearchSchema, FieldNodeSchema, AggregationNodeSchema } from './query.zod'; +import { SortNodeSchema, QuerySchema, FullTextSearchSchema, FieldNodeSchema, AggregationNodeSchema, QUERY_CURSOR_REMOVED, QUERY_DISTINCT_REMOVED } from './query.zod'; +import { retiredKey } from '../shared/retired-key'; import { ExecutionContextSchema } from '../kernel/execution-context.zod'; /** @@ -111,8 +112,8 @@ export const EngineQueryOptionsSchema = lazySchema(() => BaseEngineOptionsSchema /** Alias for limit (OData compatibility) */ top: z.number().optional(), - /** Cursor for keyset pagination */ - cursor: z.record(z.string(), z.unknown()).optional(), + /** Keyset cursor — REMOVED (#4286); same tombstone as `QuerySchema.cursor`. */ + cursor: retiredKey(QUERY_CURSOR_REMOVED), /** Full-text search configuration */ search: FullTextSearchSchema.optional(), @@ -127,8 +128,8 @@ export const EngineQueryOptionsSchema = lazySchema(() => BaseEngineOptionsSchema */ expand: z.lazy(() => z.record(z.string(), QuerySchema)).optional(), - /** SELECT DISTINCT flag */ - distinct: z.boolean().optional(), + /** SELECT DISTINCT — REMOVED (#4286); same tombstone as `QuerySchema.distinct`. */ + distinct: retiredKey(QUERY_DISTINCT_REMOVED), }).describe('QueryAST-aligned query options for IDataEngine.find() operations')); // -------------------------------------------------------------------------- @@ -273,6 +274,14 @@ export const EngineAggregateOptionsSchema = lazySchema(() => BaseEngineOptionsSc * e.g. [{ function: 'sum', field: 'amount', alias: 'total' }] */ aggregations: z.array(AggregationNodeSchema).optional(), + /** + * HAVING — a FilterCondition over the AGGREGATED rows, so its namespace is + * the aggregated row's own columns: aggregation aliases + groupBy + * projections. Enforced ENGINE-side after aggregation (#4286 step 3) — + * identical semantics on the native-driver and in-memory paths; drivers do + * not receive authority over it. + */ + having: FilterConditionSchema.optional().describe('HAVING — filter over the aggregated rows (aggregation aliases + groupBy projections); applied engine-side after aggregation'), /** * Reference timezone (IANA name) for date bucketing (ADR-0053 Phase 2). * When set to a non-UTC zone, `groupBy` items carrying a `dateGranularity` diff --git a/packages/spec/src/data/query.test.ts b/packages/spec/src/data/query.test.ts index 99d73e4567..8e546e7e0c 100644 --- a/packages/spec/src/data/query.test.ts +++ b/packages/spec/src/data/query.test.ts @@ -68,15 +68,9 @@ describe('QuerySchema - Basic', () => { expect(() => QuerySchema.parse(query)).not.toThrow(); }); - it('should accept query with distinct', () => { - const query: QueryAST = { - object: 'account', - fields: ['status'], - distinct: true, - }; - - expect(() => QuerySchema.parse(query)).not.toThrow(); - }); + // 'query with distinct' moved to the retirement pins (#4286): the tombstone + // refuses the key; unique values go through groupBy / count_distinct / the + // drivers' distinct(object, field) door. }); /** @@ -511,6 +505,42 @@ describe('QueryAST.windowFunctions — REMOVED (#4286)', () => { }); }); +describe('QueryAST.cursor — REMOVED (#4286)', () => { + it('rejects a caller-built cursor with the prescription pointing at the manual keyset', () => { + expect(() => QuerySchema.parse({ object: 'customer', cursor: { id: 'rec_9' } })) + .toThrow(/query\.cursor.*removed.*where.*sort key/s); + expect(() => QuerySchema.parse({ object: 'customer', cursor: {} })) + .toThrow(/query\.cursor.*removed/s); + }); + + it('still accepts absence, and the manual keyset parses', () => { + expect(() => QuerySchema.parse({ object: 'customer', cursor: undefined })).not.toThrow(); + expect(() => QuerySchema.parse({ + object: 'customer', + where: { created_at: { $gt: '2026-01-01T00:00:00.000Z' } }, + orderBy: [{ field: 'created_at', order: 'asc' }], + limit: 10, + })).not.toThrow(); + }); +}); + +describe('QueryAST.distinct — REMOVED (#4286)', () => { + it('rejects the flag with the prescription naming the live spellings — either value', () => { + expect(() => QuerySchema.parse({ object: 'account', distinct: true })) + .toThrow(/query\.distinct.*removed.*count_distinct/s); + // `distinct: false` was as inert as true — the tombstone refuses any value. + expect(() => QuerySchema.parse({ object: 'account', distinct: false })) + .toThrow(/query\.distinct.*removed/s); + }); + + it('per-aggregation `distinct` is a DIFFERENT, live member and still parses', () => { + expect(() => QuerySchema.parse({ + object: 'order', + aggregations: [{ function: 'count', field: 'customer_id', distinct: true, alias: 'unique_customers' }], + })).not.toThrow(); + }); +}); + describe('QuerySchema - Complex Queries', () => { it('should accept complex query with aggregations, HAVING, ordering and paging', () => { const query: QueryAST = { @@ -536,7 +566,6 @@ describe('QuerySchema - Complex Queries', () => { const query: QueryAST = { object: 'order', fields: ['id', 'customer_id', 'amount'], - distinct: true, where: { status: 'completed' }, expand: { customer_id: { object: 'customer', fields: ['name'] } }, aggregations: [ @@ -706,23 +735,6 @@ describe('QuerySchema - Type Coercion Edge Cases', () => { }); }); - it('should handle boolean flags', () => { - const query: QueryAST = { - object: 'account', - fields: ['name'], - distinct: true, - }; - - expect(() => QuerySchema.parse(query)).not.toThrow(); - - const query2: QueryAST = { - object: 'account', - fields: ['name'], - distinct: false, - }; - - expect(() => QuerySchema.parse(query2)).not.toThrow(); - }); it('should handle default sort order', () => { const query: QueryAST = { diff --git a/packages/spec/src/data/query.zod.ts b/packages/spec/src/data/query.zod.ts index 0e000d8b9d..0ecace2056 100644 --- a/packages/spec/src/data/query.zod.ts +++ b/packages/spec/src/data/query.zod.ts @@ -220,6 +220,32 @@ const QUERY_JOINS_REMOVED = + 'the engine resolves via batch $in queries, and a single related column is a dotted ' + "`fields` path (`fields: ['owner.name']`)."; +/** + * Exported (unlike the two above) because `EngineQueryOptionsSchema` + * (`data-engine.zod.ts`) re-declared both keys and tombstones them with the + * same prescription — one string, two rejection sites. + */ +export const QUERY_CURSOR_REMOVED = + '`query.cursor` was removed in @objectstack/spec 18 (#4286, ADR-0049) — no driver ever ' + + 'implemented keyset pagination, so the cursor was accepted and ignored and every page came ' + + 'back identical (a caller looping "until hasMore is false" never terminates). Delete the ' + + 'key; `QueryBuilder.cursor()` was removed with it. Express the keyset as an ordinary ' + + '`where` predicate on your sort key — `where: { created_at: { $gt: last.created_at } }` ' + + 'with the matching `orderBy` — which every driver executes with canonicalised comparands. ' + + 'A first-class cursor, if ever built, will be a response-minted opaque token, not this ' + + 'caller-built record.'; + +/** See {@link QUERY_CURSOR_REMOVED} for why this one is exported. */ +export const QUERY_DISTINCT_REMOVED = + '`query.distinct` was removed in @objectstack/spec 18 (#4286, ADR-0049 / ADR-0078) — no ' + + "driver ever rendered SELECT DISTINCT; the flag's only observable effect was MIS-WIRED: " + + 'the REST list path treated a distinct query as not countable and silently degraded ' + + '`total`/`hasMore` to a page-local estimate while still returning duplicate rows. Delete ' + + 'the key; `QueryBuilder.distinct()` was removed with it, and the count suppression is gone ' + + '(`total` is truthful again). For unique values of one column use the SQL/memory drivers\' ' + + '`distinct(object, field)` door; for unique combinations, `groupBy`; for a deduplicated ' + + 'count, the `count_distinct` aggregation.'; + const QUERY_WINDOW_FUNCTIONS_REMOVED = '`query.windowFunctions` was removed in @objectstack/spec 18 (#4286, ADR-0049) — `find()` ' + 'never applied it: no engine or driver read the key on the query path, so every OVER ' @@ -273,7 +299,12 @@ export type FullTextSearch = z.infer; * * Updates (v3): * - Added `search` parameter for full-text search (P2 requirement) - * + * + * Updates (18 — #4286, ADR-0049 enforce-or-remove): + * - `joins` / `windowFunctions` / `cursor` / `distinct` REMOVED (tombstoned; + * each rejection carries its replacement) + * - `having` ENFORCED engine-side after aggregation + * * @example * // Simple query: SELECT name, email FROM account WHERE status = 'active' * { @@ -323,7 +354,8 @@ const BaseQuerySchema = z.object({ limit: z.number().optional().describe('Max records to return (LIMIT)'), offset: z.number().optional().describe('Records to skip (OFFSET)'), top: z.number().optional().describe('Alias for limit (OData compatibility)'), - cursor: z.record(z.string(), z.unknown()).optional().describe('Cursor for keyset pagination'), + /** Keyset cursor — REMOVED (#4286): express the keyset as a `where` predicate on the sort key. */ + cursor: retiredKey(QUERY_CURSOR_REMOVED), /** Joins — REMOVED (#4286): `expand` is the one spelling for related records. */ joins: retiredKey(QUERY_JOINS_REMOVED), @@ -334,14 +366,14 @@ const BaseQuerySchema = z.object({ /** Group By Clause */ groupBy: z.array(GroupByNodeSchema).optional().describe('GROUP BY targets (strings or `{field, dateGranularity?}` objects for date bucketing)'), - /** Having Clause */ - having: FilterConditionSchema.optional().describe('HAVING clause for aggregation filtering'), - + /** Having Clause — enforced engine-side after aggregation (#4286 step 3). */ + having: FilterConditionSchema.optional().describe('HAVING — filter over the AGGREGATED rows (aggregation aliases + groupBy projections); applied engine-side after aggregation'), + /** Window functions — REMOVED from the request surface (#4286); the capability's door is `SqlDriver.findWithWindowFunctions()`. */ windowFunctions: retiredKey(QUERY_WINDOW_FUNCTIONS_REMOVED), - /** Subquery flag */ - distinct: z.boolean().optional().describe('SELECT DISTINCT flag'), + /** SELECT DISTINCT — REMOVED (#4286): `groupBy` / `count_distinct` / the drivers' `distinct()` door are the live spellings. */ + distinct: retiredKey(QUERY_DISTINCT_REMOVED), }); /** diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index dbdd0cf6f5..38df0e7751 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -597,6 +597,15 @@ const step18: MigrationStep = { + 'spec vocabulary never matched (it declared `field`/`over`/`frame` members the door never ' + 'read — that cluster goes too). Request shapes again: two semantic TODOs, no source ' + 'rewrite.\n\n' + + 'The #4286 close-out settles the remaining three. `having` is ENFORCED, not removed — ' + + 'the engine applies it after aggregation on both paths, so the clause every SQL-literate ' + + 'author expects now works (no migration; queries that carried it were silently returning ' + + 'every group and now filter as written). `cursor` and `distinct` are tombstoned WITH ' + + 'their shipped SDK producers (`QueryBuilder.cursor()` / `.distinct()` are deleted): no ' + + 'driver ever implemented keyset pagination or SELECT DISTINCT, `cursor` re-served page 1 ' + + "forever, and `distinct`'s only observable effect was mis-wired — it suppressed the REST " + + 'list count, which is now truthful again. Both are request shapes; two more semantic ' + + 'TODOs, no source rewrite.\n\n' + 'The same kind of retirement covers `wait`\'s timeout pair (#4158). `waitEventConfig.onTimeout` ' + 'had ZERO readers — no path ever inspected it, so neither `fail` nor `continue` ever ' + 'happened, while its `.default(\'fail\')` stamped a decision nothing made onto every wait ' @@ -691,6 +700,50 @@ const step18: MigrationStep = { + "driver's `findWithWindowFunctions` door directly. A query that still carries the key " + 'fails to parse with the removal prescription naming that door.', }, + { + id: 'query-cursor-retired', + surface: 'data.query.cursor', + replacement: + 'a `where` predicate on the sort key — `where: { created_at: { $gt: last.created_at } }` ' + + 'with the matching `orderBy` (the documented manual-keyset pattern)', + reason: + 'The `cursor` key promised keyset pagination and no driver implemented it: the cursor ' + + 'was accepted and ignored, so every page came back identical — a caller looping ' + + '"until hasMore is false" never terminates. Worse than inert, it had a shipped public ' + + 'producer (`QueryBuilder.cursor()`, removed with the key). The caller-built ' + + '`Record` shape also leaks sort/storage detail and squats on the ' + + 'reserved REST parameter set; a first-class cursor, if ever designed, will be a ' + + 'response-minted opaque token — a different API, so keeping this one preserved a ' + + 'wrong design rather than a roadmap. A REQUEST surface, never stored; nothing to ' + + 'rewrite. ADR-0049 / ADR-0078, #4286.', + acceptanceCriteria: + 'No caller sends `cursor` and no SDK call site uses `QueryBuilder.cursor()`; deep ' + + 'pagination expresses the keyset as a `where` predicate on the sort key. A query ' + + 'still carrying `cursor` fails to parse with the removal prescription, and authoring ' + + 'it is a `tsc` error.', + }, + { + id: 'query-distinct-retired', + surface: 'data.query.distinct', + replacement: + '`groupBy` for unique combinations; the `count_distinct` aggregation for deduplicated ' + + "counts; the SQL/memory drivers' `distinct(object, field)` door for one column's values", + reason: + 'The `distinct` flag promised SELECT DISTINCT and no driver ever rendered it — but it ' + + 'was MIS-WIRED rather than merely dead (the harsher ADR-0078 class): the REST list ' + + 'path treated a distinct query as not countable and silently degraded ' + + '`total`/`hasMore` to a page-local estimate, so the caller got duplicate rows AND ' + + 'worse pagination metadata, and a side effect that "confirmed" the flag was doing ' + + 'something. It had a shipped public producer (`QueryBuilder.distinct()`, removed with ' + + 'the key). The count suppression is deleted in the same change — `total` is truthful ' + + 'for those queries again. A REQUEST surface, never stored; nothing to rewrite. ' + + 'ADR-0049 / ADR-0078, #4286.', + acceptanceCriteria: + 'No caller sends `distinct` and no SDK call site uses `QueryBuilder.distinct()`; ' + + 'deduplication goes through `groupBy` / `count_distinct` / the drivers\' `distinct()` ' + + 'door. A query still carrying the key fails to parse with the removal prescription, ' + + 'and the REST list response reports a real `total` for queries that used to send it.', + }, ], }; diff --git a/skills/objectstack-query/SKILL.md b/skills/objectstack-query/SKILL.md index f8f095b420..26de4c66ab 100644 --- a/skills/objectstack-query/SKILL.md +++ b/skills/objectstack-query/SKILL.md @@ -278,10 +278,11 @@ Sort with `orderBy` — an array of sort nodes: ### Keyset Pagination (Performant) -> ⚠️ **`cursor` is schema-reserved — NOT executed by the engine yet.** The -> `cursor` property validates against `QuerySchema`, but no engine or driver -> code reads it — a query carrying `cursor` silently returns **page 1 -> forever**. Do keyset pagination manually with `where` + `orderBy` + `limit`: +> ⛔ **`query.cursor` was REMOVED in `@objectstack/spec` 18 (#4286).** No +> engine or driver ever read it — a query carrying `cursor` silently returned +> **page 1 forever**. The key is tombstoned (a query carrying it fails to +> parse with the prescription) and `QueryBuilder.cursor()` is gone. Do keyset +> pagination with `where` + `orderBy` + `limit`: ```typescript // First page @@ -363,24 +364,21 @@ unique or near-unique column such as `created_at` or `id`) so ### HAVING Clause -> ⚠️ **Schema-reserved — NOT executed by the engine yet.** `having` -> validates against `QuerySchema`, but `EngineAggregateOptions` has no -> `having` property and nothing implements it — the clause is silently -> dropped. **Working alternative:** post-filter the aggregated rows in -> application code: +> ✅ **Enforced since #4286.** The engine applies `having` AFTER aggregation, +> on both the native-driver path and the in-memory fallback. It references +> the **aggregated row's columns** — aggregation aliases and groupBy +> projections — with the ordinary FilterCondition operators plus +> `$and`/`$or`/`$not`. An unknown operator is rejected loudly, never ignored. ```typescript -// ❌ having is silently ignored — do NOT rely on it -// { ..., having: { total_revenue: { $gt: 100000 } } } - -// ✅ Aggregate, then filter the result rows in app code +// ✅ Only regions with more than 100k revenue const rows = await engine.aggregate('deal', { groupBy: ['region'], aggregations: [ { function: 'sum', field: 'amount', alias: 'total_revenue' }, ], + having: { total_revenue: { $gt: 100000 } }, }); -const bigRegions = rows.filter((r) => r.total_revenue > 100000); ``` ### Filtered Aggregation diff --git a/skills/objectstack-query/evals/README.md b/skills/objectstack-query/evals/README.md index b34b5beac9..b92e167e13 100644 --- a/skills/objectstack-query/evals/README.md +++ b/skills/objectstack-query/evals/README.md @@ -15,7 +15,7 @@ subset the engine actually executes. not a `joins` array (removed in #4286). 3. **Pagination pattern** — "Implement infinite scroll for a feed." Expect manual keyset pagination (`where` on the sort key + `orderBy` + `limit`); - fail if the answer uses the schema-reserved `cursor` property. + fail if the answer uses the removed `cursor` property (#4286). 4. **Aggregation correctness** — "Count deals by region and show total revenue." Expect `groupBy` + `count`/`sum` with aliases; on SQL targets the answer must stay within `count`/`sum`/`avg`/`min`/`max`. diff --git a/skills/objectstack-query/rules/aggregation.md b/skills/objectstack-query/rules/aggregation.md index 5b09784a3e..98157cd7fe 100644 --- a/skills/objectstack-query/rules/aggregation.md +++ b/skills/objectstack-query/rules/aggregation.md @@ -86,26 +86,23 @@ aggregations (never bucket by hand in app code): ## HAVING Clause -> ⚠️ **Schema-reserved — NOT executed by the engine yet.** `having` -> validates against `QuerySchema`, but `EngineAggregateOptions` has no -> `having` property and nothing implements it — the clause is silently -> dropped and every group is returned. **Working alternative:** post-filter -> the aggregated rows in application code: +> ✅ **Enforced since #4286.** The engine applies `having` AFTER aggregation +> (both the native-driver path and the in-memory fallback), referencing the +> **aggregated row's columns** — aggregation aliases and groupBy projections. +> Ordinary FilterCondition operators plus `$and`/`$or`/`$not`; an unknown +> operator is rejected loudly, never ignored. ```typescript -// ❌ having is silently ignored -// { ..., having: { order_count: { $gt: 5 } } } - -// ✅ Aggregate, then filter the result rows in app code +// ✅ Only customers with more than 5 orders const rows = await engine.aggregate('order', { groupBy: ['customer_id'], aggregations: [{ function: 'count', alias: 'order_count' }], + having: { order_count: { $gt: 5 } }, }); -const frequentCustomers = rows.filter((r) => r.order_count > 5); ``` -Aggregated result sets are one row per group — usually small enough that an -app-side filter is cheap. Use `where` to shrink the input rows first. +`having` filters *groups*; `where` filters the *input rows* before grouping — +use `where` to shrink the scan, `having` to threshold the aggregates. ## Filtered Aggregation (FILTER WHERE) @@ -242,15 +239,12 @@ over the two periods and join the buckets in app code. groupBy: ['customer_id'] } -// ❌ Also wrong: having is schema-reserved and silently ignored (see above) -// { ..., having: { order_count: { $gt: 5 } } } - -// ✅ Aggregate, then filter the group rows in app code +// ✅ Right: `having` references the aggregation ALIAS, after grouping (#4286) const rows = await engine.aggregate('order', { groupBy: ['customer_id'], aggregations: [{ function: 'count', alias: 'order_count' }], + having: { order_count: { $gt: 5 } }, }); -const result = rows.filter((r) => r.order_count > 5); ``` ### ❌ Wrong: sum/avg on non-numeric fields diff --git a/skills/objectstack-query/rules/pagination.md b/skills/objectstack-query/rules/pagination.md index 1100575dd5..6cb5312dde 100644 --- a/skills/objectstack-query/rules/pagination.md +++ b/skills/objectstack-query/rules/pagination.md @@ -9,11 +9,12 @@ Guide for implementing pagination in ObjectStack queries. | Offset | UI page navigation, small datasets | Simple, random page access | Slow on large offsets, drift on inserts | | Keyset (manual `where`) | Infinite scroll, real-time feeds | Consistent results, O(1) performance | No random page access | -> ⚠️ **The `cursor` query property is schema-reserved — NOT executed by the -> engine yet.** It validates against `QuerySchema`, but no engine or driver -> code reads it: a query carrying `cursor` silently returns **page 1 -> forever**. Implement keyset pagination manually with a `where` filter on -> the sort key (pattern below). +> ⛔ **The `cursor` query property was REMOVED in `@objectstack/spec` 18 +> (#4286).** No engine or driver ever read it: a query carrying `cursor` +> silently returned **page 1 forever**. The key is tombstoned — a query +> carrying it fails to parse with the prescription — and +> `QueryBuilder.cursor()` is gone. Implement keyset pagination with a +> `where` filter on the sort key (pattern below). ## Offset Pagination @@ -50,7 +51,7 @@ Guide for implementing pagination in ObjectStack queries. ## Keyset Pagination (Manual) Keyset pagination uses the last record's sort key value to fetch the next -page. Because the `cursor` property is not executed (see above), express the +page. Because the `cursor` property was removed (see above), express the keyset as a `where` filter on the sort field: ```typescript @@ -156,10 +157,10 @@ When building paginated REST endpoints: ## Common Mistakes -### ❌ Wrong: Using the schema-reserved `cursor` property +### ❌ Wrong: Using the removed `cursor` property ```typescript -// ❌ cursor is never read — this returns page 1 forever +// ❌ cursor was removed in #4286 — the tombstone rejects this query outright { object: 'post', limit: 20,