Skip to content

Commit b09d8d9

Browse files
os-zhuangclaude
andauthored
feat(objectql,spec,client,metadata-protocol)!: #4286 收官 — having 落地执行,cursor/distinct 连同 SDK 生产者移除 (#4307)
* feat(objectql,spec,metadata-protocol)!: enforce query.having — the engine applies it after aggregation (#4286 step 3) ADR-0049 resolved to ENFORCE: having was the one declared member every SQL-literate author expects to work next to groupBy/aggregations, and its gap was structural (finding 1 — aggregate() rebuilt the driver AST without it, and the findData aggregate branch dropped it on the wire). - applyHaving() (objectql/src/having-filter.ts) runs AFTER aggregation on the native-driver path and the in-memory fallback alike; namespace is the aggregated row's own columns (aggregation aliases + groupBy projections). An unknown operator rejects loudly — ignoring one would silently return unfiltered aggregates, the ADR-0078 failure enforcement exists to end. - EngineAggregateOptionsSchema declares having; findData's aggregate branch forwards it; the ast carries it so the FLS predicate guard (which already walked having references) sees caller input. - Native SQL HAVING pushdown can come later behind a driver capability flag (the dateGranularity two-tier pattern) without changing semantics. - Ledger: query.having flips dead → live with end-to-end evidence; docs and the objectstack-query skill stop teaching the app-code post-filter workaround. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012K2BX3WQwPBA2ZBehqJUBX * refactor(spec,client,metadata-protocol)!: remove query.cursor and query.distinct with their SDK producers (#4286 step 4) ADR-0049 resolved both to REMOVE. Neither ever had an executor: - cursor promised keyset pagination no driver implemented — accepted and ignored, every page identical, so a caller looping "until hasMore" never terminated. Tombstoned on QuerySchema and EngineQueryOptionsSchema (one shared prescription); QueryBuilder.cursor() deleted. The 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. The cursor params on listRevisions/flow-runs/notifications are those endpoints' own live tokens — untouched. - distinct was MIS-WIRED, not merely dead (finding 2): its only observable effect was suppressing the REST list count, so callers got duplicate rows AND degraded total/hasMore — a side effect that "confirmed" a capability that never ran. Tombstoned on both schemas plus the ?distinct querystring spelling (HttpFindQueryParamsSchema); QueryBuilder.distinct() deleted; the countable suppression branch deleted — total is truthful again (the observable REST change, carried in the changeset). AggregationNode.distinct (per-aggregation dedupe) is a different, live member and stays. Both register as protocol-18 semantic migrations (query-cursor-retired, query-distinct-retired) — request shapes, nothing stored to rewrite. Ledger entries flip to REMOVED notes (rows stay: retiredKey keeps each key in the walked shape); docs and the objectstack-query skill teach the live spellings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012K2BX3WQwPBA2ZBehqJUBX * docs(deployment): the troubleshooting pagination example stops teaching the removed cursor key (#4286) Found via the docs-drift advisory on #4307: the 'Query is slow' checklist still demonstrated cursor: { id: lastSeenId } — the key the same PR tombstones. The example now expresses the keyset as a where predicate on the sort key, matching the pattern every other page teaches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012K2BX3WQwPBA2ZBehqJUBX --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e5a4d26 commit b09d8d9

33 files changed

Lines changed: 864 additions & 205 deletions

.changeset/query-cursor-removed.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
"@objectstack/spec": major
3+
"@objectstack/client": major
4+
---
5+
6+
refactor(data)!: `query.cursor` is removed — no driver ever implemented keyset pagination (#4286 step 4)
7+
8+
`cursor` promised keyset pagination and nothing served it: the key was accepted
9+
and ignored, so every page came back identical — a caller looping "until
10+
`hasMore` is false" never terminated. It was Tier A of the #4286 inventory: a
11+
shipped public producer (`QueryBuilder.cursor()`) minting a key no executor
12+
read.
13+
14+
**FROM → TO**
15+
16+
| Was | Now |
17+
| :--- | :--- |
18+
| `cursor: { created_at: last.created_at }` | `where: { created_at: { $gt: last.created_at } }` + the matching `orderBy` |
19+
| `QueryBuilder.cursor({...})` | `.where({ created_at: { $gt: ... } }).orderBy('created_at')` |
20+
21+
The one-line fix: **delete the key and seek with `where` on your sort key**
22+
every driver already executes that, with canonicalised temporal comparands.
23+
24+
Mechanics: `retiredKey()` tombstones on both declaration sites
25+
(`QuerySchema.cursor` and `EngineQueryOptionsSchema.cursor`, one shared
26+
prescription), so authoring the key fails `tsc` and a query still carrying it
27+
fails to parse with the fix. `QueryBuilder.cursor()` is deleted. Registered as
28+
the protocol-18 semantic migration `query-cursor-retired` (request surface —
29+
nothing stored to rewrite). The caller-built `Record<string, unknown>` shape
30+
would not survive a real keyset design anyway: a first-class cursor, if ever
31+
built, will be a response-minted opaque token (the pattern the
32+
metadata-revision / flow-run / notification list endpoints already use — those
33+
`cursor` params are unrelated and unchanged).
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
"@objectstack/spec": major
3+
"@objectstack/client": major
4+
"@objectstack/metadata-protocol": minor
5+
---
6+
7+
refactor(data)!: `query.distinct` is removed, and with it the mis-wired REST count suppression (#4286 step 4)
8+
9+
`distinct` promised `SELECT DISTINCT` and no driver ever rendered it — but it
10+
was **mis-wired rather than merely dead** (#4286 finding 2, the harsher
11+
ADR-0078 class): its only observable effect platform-wide was that the REST
12+
list path treated a distinct query as *not countable*, silently degrading
13+
`total`/`hasMore` to a page-local estimate while still returning duplicate
14+
rows. A caller — or a self-verifying agent — saw the response change and
15+
concluded the flag worked. It had a shipped public producer
16+
(`QueryBuilder.distinct()`).
17+
18+
**FROM → TO**
19+
20+
| Was | Now |
21+
| :--- | :--- |
22+
| `distinct: true` for unique combinations | `groupBy: ['category']` |
23+
| `distinct: true` + count | `aggregations: [{ function: 'count_distinct', field: 'category', alias: '...' }]` |
24+
| one column's distinct values | the SQL/memory drivers' `distinct(object, field)` door (driver-level) |
25+
26+
The one-line fix: **delete the key**; deduplicate with `groupBy` /
27+
`count_distinct`.
28+
29+
Mechanics: `retiredKey()` tombstones on both declaration sites
30+
(`QuerySchema.distinct` and `EngineQueryOptionsSchema.distinct`, one shared
31+
prescription); `QueryBuilder.distinct()` is deleted; registered as the
32+
protocol-18 semantic migration `query-distinct-retired`. **Observable REST
33+
change (`@objectstack/metadata-protocol`):** the count-suppression branch is
34+
deleted — a list request that used to carry `distinct` now gets a real
35+
`total`/`hasMore` again (that restoration is the point, not a side effect).
36+
The per-aggregation `distinct` flag (`AggregationNode.distinct`) is a
37+
different, live member and is untouched.
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/objectql": minor
4+
"@objectstack/metadata-protocol": minor
5+
---
6+
7+
feat(objectql)!: `query.having` is enforced — the engine applies it after aggregation (#4286 step 3, ADR-0049 resolved to enforce)
8+
9+
`having` had been declared on the request surface since AST v2 and executed by
10+
nothing. #4286 finding 1 showed the gap was structural: `engine.aggregate()`
11+
rebuilt the driver AST with exactly `object`/`where`/`groupBy`/`aggregations`,
12+
so even a driver that *did* implement HAVING could never have received it, and
13+
the one wire path (`findData`'s aggregate branch) dropped the clause too. It
14+
was the strongest enforce candidate of the #4286 set — the clause every
15+
SQL-literate author (human or model) expects to work next to
16+
`groupBy`/`aggregations` — and it is now live end to end:
17+
18+
- **Engine-owned, both paths.** `applyHaving()`
19+
(`packages/objectql/src/having-filter.ts`) runs AFTER aggregation on the
20+
native-driver path and the in-memory fallback alike — the same
21+
correct-first / optimize-later two-tier shape date bucketing uses. Native
22+
SQL `HAVING` pushdown can come later behind a driver capability flag without
23+
changing semantics.
24+
- **Namespace: the aggregated row's own columns** — aggregation aliases
25+
(`order_count`, `total`) and groupBy projections — with the ordinary
26+
FilterCondition operators plus `$and`/`$or`/`$not`.
27+
- **An unknown operator rejects loudly.** Ignoring one (as tolerant matchers
28+
do) would silently return unfiltered aggregates — the exact ADR-0078
29+
silently-inert failure enforcement exists to end.
30+
- **The wire path forwards it.** `findData`'s aggregate branch passes
31+
`having` through, and `EngineAggregateOptionsSchema` now declares it.
32+
- The FLS predicate guard already walked `having` references
33+
(`predicate-guard.ts`), which is what made enforcement safe to turn on.
34+
35+
No migration needed: queries that carried `having` before were silently
36+
returning every group; they now filter as written. A caller who depended on
37+
the clause being *ignored* (sending `having` and expecting unfiltered
38+
results) sees the corrected behavior — that is the enforcement, not a
39+
regression.

content/docs/data-modeling/queries.mdx

Lines changed: 37 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -239,23 +239,24 @@ reaching `engine.find()` directly are unaffected.
239239
}
240240
```
241241

242-
### Cursor-Based Pagination (Keyset) — not implemented
242+
### Keyset Pagination — a `where` predicate on the sort key
243+
244+
`query.cursor` was **removed in `@objectstack/spec` 18** (#4286): nothing on the server
245+
ever read it, so a cursor query silently returned the same first page every time. The
246+
key is tombstoned and `QueryBuilder.cursor()` was removed with it. Express the keyset
247+
directly — seek past the last row instead of offsetting:
243248

244249
```typescript
250+
// Next page after `last` — every driver executes this, with canonicalised comparands
245251
{
246-
limit: 20,
247-
cursor: { id: 100 } // Opaque keyset cursor (e.g. last seen sort-key values)
252+
where: { created_at: { $gt: last.created_at } },
253+
orderBy: [{ field: 'created_at', order: 'asc' }],
254+
limit: 20
248255
}
249256
```
250257

251-
<Callout type="warn">
252-
`cursor` is declared on `QuerySchema` as an opaque record (`Record<string, unknown>`)
253-
and `QueryBuilder.cursor()` will set it on a query, but **nothing on the server reads
254-
it** — the query engine, the REST query dispatcher, and the SQL / in-memory / MongoDB
255-
drivers all ignore it, so a cursor query silently returns the same first page every
256-
time. Use `limit` + `offset` until keyset pagination is wired up. (There is no
257-
`keyset`/`after` query property either.)
258-
</Callout>
258+
(A first-class cursor, if ever designed, will be a response-minted opaque token — the
259+
pattern the metadata-revision / flow-run / notification list endpoints already use.)
259260

260261
---
261262

@@ -378,13 +379,14 @@ silently returns `null` for them. Avoid these three on SQL- or memory-backed obj
378379
}
379380
```
380381

381-
<Callout type="warn">
382-
`having` is accepted by `QuerySchema` but is not enforced by the query engine today.
383-
`EngineAggregateOptions` (the type `ObjectQL.aggregate()` actually takes) has no `having`
384-
field, and both the REST `findData()` dispatcher and `ObjectQL.aggregate()` build their
385-
driver-facing query from only `where` / `groupBy` / `aggregations` — a `having` clause is
386-
silently dropped before it reaches any driver. No driver (SQL, in-memory, MongoDB) filters
387-
on it either. Post-filter grouped results on the client until this is wired up.
382+
<Callout type="info">
383+
`having` is **enforced since #4286** (ADR-0049 enforce-or-remove, resolved to enforce): the
384+
engine applies it itself AFTER aggregation, identically on the native-driver path and the
385+
in-memory fallback, and the REST `findData()` aggregate branch forwards it. Its namespace is
386+
the **aggregated row's own columns** — aggregation aliases (`order_count`) and groupBy
387+
projections — with the ordinary FilterCondition operators and `$and`/`$or`/`$not`. An
388+
unknown operator is rejected loudly rather than ignored. Native SQL `HAVING` pushdown can
389+
come later behind a driver capability flag without changing these semantics.
388390
</Callout>
389391

390392
---
@@ -507,22 +509,24 @@ report/dashboard metadata.
507509

508510
## Distinct & Group By
509511

510-
### Distinct Records
512+
### Distinct Records — removed flag, three live spellings
513+
514+
The top-level `query.distinct` flag was **removed in `@objectstack/spec` 18** (#4286):
515+
no driver's `find()` ever applied it, and its only observable effect was mis-wired —
516+
it silently suppressed the REST list count while still returning duplicate rows (the
517+
count is truthful again). The key is tombstoned and `QueryBuilder.distinct()` was
518+
removed with it. What actually deduplicates:
511519

512520
```typescript
513-
{
514-
object: 'task',
515-
fields: ['category'],
516-
distinct: true
517-
}
521+
// Unique combinations → groupBy
522+
{ object: 'task', groupBy: ['category'] }
523+
524+
// Deduplicated count → count_distinct
525+
{ object: 'task', aggregations: [{ function: 'count_distinct', field: 'category', alias: 'categories' }] }
518526
```
519527

520-
<Callout type="warn">
521-
The top-level `distinct: true` flag is defined in `QuerySchema` but isn't applied by any
522-
driver's `find()` (SQL, in-memory, and MongoDB all ignore it). A separate
523-
`driver.distinct(object, field)` method exists on the SQL and in-memory drivers, but it isn't
524-
called by `ObjectQL.find()`/`.aggregate()`, so it isn't reachable through a normal query.
525-
</Callout>
528+
A separate `driver.distinct(object, field)` method also exists on the SQL and
529+
in-memory drivers (driver-level; not called by `ObjectQL.find()`/`.aggregate()`).
526530

527531
### Group By with Having
528532

@@ -538,9 +542,9 @@ called by `ObjectQL.find()`/`.aggregate()`, so it isn't reachable through a norm
538542
}
539543
```
540544

541-
<Callout type="warn">
542-
As noted under [Aggregations](#aggregations) above, `having` is not currently enforced —
543-
it's dropped before reaching the aggregation engine or any driver.
545+
<Callout type="info">
546+
As noted under [Aggregations](#aggregations) above, `having` filters the aggregated rows
547+
engine-side — `total_spent` here is the aggregation alias it references.
544548
</Callout>
545549

546550
### Date Bucketing in `groupBy`

content/docs/deployment/troubleshooting.mdx

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -278,17 +278,22 @@ console.log(field.maxLength?.toString() ?? 'no limit');
278278
2. **Limit fields** — Only request fields you need (`fields: ['id', 'name']`)
279279
3. **Use pagination** — Always set `limit` to avoid returning all records
280280
4. **Avoid deep nesting** — Limit nested `$and`/`$or` depth
281-
5. **Use cursor pagination** — For large datasets, cursor-based is faster than offset
281+
5. **Use keyset pagination** — For large datasets, seeking past the last row is
282+
faster than a deep `offset`. Express the keyset as a `where` predicate on the
283+
sort key (the `cursor` query property was removed in `@objectstack/spec` 18,
284+
#4286 — nothing ever read it)
282285

283286
```typescript
284-
// Optimized query
287+
// Optimized query — next page seeks past the last row instead of offsetting
285288
{
286289
object: 'activity',
287-
fields: ['id', 'type', 'created_at'], // Only needed fields
288-
where: { type: { $eq: 'login' } }, // On indexed field
290+
fields: ['id', 'type', 'created_at'], // Only needed fields
291+
where: {
292+
type: { $eq: 'login' }, // On indexed field
293+
created_at: { $lt: lastSeenCreatedAt }, // Keyset: past the last row
294+
},
289295
orderBy: [{ field: 'created_at', order: 'desc' }],
290296
limit: 25,
291-
cursor: { id: lastSeenId } // Keyset/cursor pagination (field → last-seen value)
292297
}
293298
```
294299

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

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,9 @@ interface QueryAST {
6969
limit?: number; // Max records (LIMIT)
7070
offset?: number; // Skip records (OFFSET)
7171
top?: number; // Alias for limit (OData compat)
72-
cursor?: Record<string, unknown>; // Keyset pagination cursor
7372
aggregations?: AggregationNode[]; // Aggregation functions
7473
groupBy?: GroupByNode[]; // GROUP BY targets (string | object)
75-
having?: FilterCondition; // HAVING clause
76-
distinct?: boolean; // SELECT DISTINCT
74+
having?: FilterCondition; // HAVING — engine-enforced after aggregation
7775
expand?: Record<string, QueryAST>; // Recursive relation loading
7876
}
7977
```
@@ -90,22 +88,21 @@ on the `find()` path:
9088

9189
| Member | Status |
9290
|:-------|:-------|
93-
| `having` | Never read: `engine.aggregate()` forwards only `object`/`where`/`groupBy`/`aggregations`, and the in-memory fallback has no HAVING stage |
94-
| `cursor` | Accepted by `EngineQueryOptions`, but no driver implements keyset pagination |
95-
| `distinct` | Not applied by `find()`; the SQL and in-memory drivers expose a separate `distinct(object, field, filters?)` method instead |
9691
| `aggregations[].filter` | `[EXPERIMENTAL — not enforced]` — a SQL `FILTER (WHERE …)` affordance neither the SQL builders nor the in-memory fallback applies |
9792
| `search.fuzzy` / `boost` / `operator` / `minScore` / `language` / `highlight` | `[EXPERIMENTAL — not enforced]` — only `query` and `fields` drive the expansion |
9893

9994
`top` is the exception that *is* honored — the engine normalises it to `limit`.
10095

101-
Two members left this table by being **removed** (#4286, ADR-0049 enforce-or-remove):
102-
`joins` and `windowFunctions` are tombstoned in `@objectstack/spec` 18 — a query
103-
carrying either fails to parse with the upgrade prescription, and authoring one is a
104-
`tsc` error. Related records are read through `expand`; window functions remain a
105-
SQL-driver door (`SqlDriver.findWithWindowFunctions()`). The inert members that
106-
*remain* declared (`having`, `cursor`, `distinct`, and the experimental flags above)
107-
are tracked in the liveness ledger (`packages/spec/liveness/query.json`) pending
108-
their #4286 dispositions.
96+
The #4286 sweep (ADR-0049 enforce-or-remove) settled every other declared-but-inert
97+
member. **Removed** — tombstoned in `@objectstack/spec` 18, so a query carrying one
98+
fails to parse with the upgrade prescription and authoring it is a `tsc` error:
99+
`joins` (related records are read through `expand`), `windowFunctions` (a SQL-driver
100+
door remains: `SqlDriver.findWithWindowFunctions()`), `cursor` (express the keyset as
101+
a `where` predicate on the sort key — §7), and `distinct` (unique values via
102+
`groupBy` / `count_distinct` / the drivers' `distinct()` door; its only observable
103+
effect was suppressing the REST list count, which is truthful again). **Enforced**:
104+
`having` (§5). The experimental flags above are tracked in the liveness ledger
105+
(`packages/spec/liveness/query.json`).
109106
</Callout>
110107

111108
### Key Types
@@ -659,23 +656,30 @@ const query: QueryAST = {
659656
};
660657
```
661658

662-
### HAVING Clause (protocol only)
659+
### HAVING Clause
663660

664-
`QuerySchema` defines a `having` property, but **nothing executes it**:
665-
`engine.aggregate()` forwards only `object` / `where` / `groupBy` / `aggregations` to
666-
the driver, no driver reads `query.having`, and the in-memory aggregation fallback has
667-
no HAVING stage. Filter the aggregated rows in application code:
661+
**Enforced since #4286** (ADR-0049, resolved to enforce). The engine applies `having`
662+
itself, AFTER aggregation, identically on the native-driver path and the in-memory
663+
fallback (`packages/objectql/src/having-filter.ts`) — the same correct-first /
664+
optimize-later two-tier shape date bucketing uses; native SQL `HAVING` pushdown can come
665+
later behind a driver capability flag without changing these semantics. The REST
666+
`findData()` aggregate branch forwards the clause.
667+
668+
`having` references the **aggregated row's own columns** — aggregation aliases and
669+
groupBy projections — with the ordinary FilterCondition operators and
670+
`$and` / `$or` / `$not`. An unknown operator rejects the query loudly rather than being
671+
ignored (an ignored operator would silently return unfiltered aggregates — the ADR-0078
672+
failure mode enforcement exists to end).
668673

669674
```typescript
675+
// Only accounts with > $1M pipeline
670676
const rows = await engine.aggregate('opportunity', {
671677
groupBy: ['account_id'],
672678
aggregations: [
673679
{ function: 'sum', field: 'amount', alias: 'total' },
674680
],
681+
having: { total: { $gt: 1_000_000 } },
675682
});
676-
677-
// Only accounts with > $1M pipeline
678-
const bigAccounts = rows.filter((r) => r.total > 1_000_000);
679683
```
680684

681685
### Date Bucketing
@@ -705,8 +709,14 @@ the driver's raw rows.
705709

706710
### Distinct
707711

708-
The `distinct` flag on `QueryAST` is **not applied by `find()`**. Distinct values come
709-
from the driver's own `distinct()` method (implemented by the SQL and in-memory drivers;
712+
`query.distinct` was **removed in `@objectstack/spec` 18** (#4286): no driver ever
713+
rendered `SELECT DISTINCT`, and the flag's only observable effect was mis-wired — it
714+
silently suppressed the REST list count (`total`/`hasMore` degraded to a page-local
715+
estimate) while still returning duplicate rows. The key is tombstoned and
716+
`QueryBuilder.distinct()` was removed with it; the count suppression is gone, so
717+
`total` is truthful again. Unique *combinations* come from `groupBy`, deduplicated
718+
counts from the `count_distinct` aggregation, and one column's distinct values from
719+
the driver's own `distinct()` method (implemented by the SQL and in-memory drivers;
710720
it is not part of the `IDataDriver` contract):
711721

712722
```typescript
@@ -823,9 +833,13 @@ const page2 = await engine.find('customer', {
823833
### Keyset Pagination
824834

825835
<Callout type="warn">
826-
`cursor` is accepted by `QuerySchema` and `EngineQueryOptions`, but **no driver
827-
implements keyset pagination** — passing it has no effect. Express the keyset yourself
828-
as an ordinary `where` predicate on the sort key:
836+
`query.cursor` was **removed in `@objectstack/spec` 18** (#4286): no driver ever
837+
implemented keyset pagination, so a cursor was accepted and ignored and every page came
838+
back identical — a caller looping "until `hasMore` is false" never terminated. The key
839+
is tombstoned (on `EngineQueryOptions` too) and `QueryBuilder.cursor()` was removed
840+
with it. Express the keyset as an ordinary `where` predicate on the sort key — the
841+
pattern below is the supported one; a first-class cursor, if ever designed, will be a
842+
response-minted opaque token:
829843
</Callout>
830844

831845
```typescript

0 commit comments

Comments
 (0)