Skip to content

[P2] data: QueryAST declares 12 members no executor runs — the liveness ledger governs metadata types, not the request surface #4286

Description

@os-zhuang

Follow-up to #4196 (merged as #4268). That issue removed one declared-but-inert member from FieldNode. Applying the same method to the rest of the request surfaceQueryAST and its satellites — turns up 12 more, and, more importantly, the reason none of the existing gates found them.

Refs #4171, #4196, ADR-0049 (enforce-or-remove), ADR-0078 (no-silently-inert), ADR-0087 (conversion/tombstone).

The inventory

BaseQuerySchema (packages/spec/src/data/query.zod.ts:581) declares 16 members. Five of them, plus six sub-flags of search and one of AggregationNode, are never executed. engine.ts contains zero reads of any of them on the query path.

Tier A — a public SDK method mints them. This is strictly worse than the #4196 case: FieldNode's object form at least had no producer.

Member Declared Producer Executor
cursor query.zod.ts:601, re-declared on EngineQueryOptionsSchema (data-engine.zod.ts) QueryBuilder.cursor()client/src/query-builder.ts:294; serialized to the wire at 5 sites in client/src/index.ts none — no driver implements keyset pagination
distinct query.zod.ts:619, also on EngineQueryOptionsSchema QueryBuilder.distinct()client/src/query-builder.ts:302 not SELECT DISTINCT — see below, it does something else
search.fuzzy query.zod.ts:524 QueryBuilder.search()'s own options type — client/src/query-builder.ts:286 none — expansion reads only query + fields

Tier B — declared, no producer, no executor.

Member Declared
joins query.zod.ts:604
having query.zod.ts:613
windowFunctions query.zod.ts:616
search.operator / boost / minScore / language / highlight query.zod.ts:525-529
AggregationNode.filter query.zod.ts:131

Four things the docs table does not say

content/docs/protocol/objectql/query-syntax.mdx:92-101 already carries an accurate callout table for most of this, and §§ "HAVING Clause (protocol only)", "Joins (protocol only)", "Window Functions", "Keyset Pagination" spell it out in prose. That is the starting point, not the finding — per ADR-0049, documenting inertness is not a disposition. Verification turned up four things the docs do not state:

1. having is not "no driver reads it" — the engine structurally cannot deliver it. engine.aggregate() rebuilds the AST it hands to the driver with exactly four keys:

ast: { object, where: query.where, groupBy: query.groupBy as any, aggregations: query.aggregations }

So a driver that did implement HAVING would still never receive it. The fix is one line larger than the docs imply.

2. distinct is not inert — it has a real effect, unrelated to its name. metadata-protocol/src/protocol.ts:3816:

const countable = options.search == null && options.distinct == null;

A distinct: true query is treated as not countable, so the REST list response skips engine.count() and degrades total/hasMore to a page-local estimate. The caller gets non-distinct rows and worse pagination metadata. Calling this one dead in a ledger would be wrong; it is mis-wired, which ADR-0078 treats more harshly, not less.

3. The security layer believes these clauses are live; no executor does. plugins/plugin-security/src/predicate-guard.ts:61-109 deliberately walks having (:64), windowFunctions including over.partitionBy / over.orderBy (:93-109), and aggregations[].filter (:89) to collect field references for FLS. Its doc comment lists them as "row-shaping clauses". Two consequences: the guard is already correct if any of these is ever made live, and removing them makes that code dead — a coupled change to track in whichever direction this goes.

4. Every one of these names is reserved at the REST boundary, and removal is therefore behavior-changing. metadata-protocol/src/protocol.ts:781:

const QUERY_AST_KEYS: Readonly<Record<keyof QueryAST, true>> = {  joins: true, having: true, windowFunctions: true, cursor: true, distinct: true  };

It feeds RESERVED_LIST_QUERY_PARAMS, which decides "query parameter vs. field filter". Its own comment names the price: "an object with a field genuinely called e.g. count or cursor must filter it through the explicit form". So these members are not free — they squat on the tenant field-name namespace for capabilities that do not work. Conversely, removing one un-reserves that name, so a field called distinct or having would start resolving as an implicit filter. That is a real compat note for the removal route, in the opposite direction from the usual one.

The same declaration is a helper for the work: it is typed Record<keyof QueryAST, true>, pinned in both directions, so dropping a key from the AST surfaces here as an excess-property error — the same compile-time guidance that made #4268 mechanical.

Why no existing gate catches this

  • Liveness ledger (packages/spec/liveness/*.json) reads BUILTIN_METADATA_TYPE_SCHEMAS — the 16 registered metadata types. QueryAST is authorable (exported from @objectstack/spec/data, built by the client SDK, accepted as the POST /data/:object/query body) but it is not a metadata type, so it is out of scope. The ledger governs what authors write into metadata files; nothing governs what callers write into a query.
  • check:exported-any (fix(spec): five exported symbols resolved to any — type the recursive schemas and gate it in CI (#4171) #4194) is a type-resolution gate. A declared-but-unproduced shape resolves fine.
  • check:authorable-surface walks the metadata-type schemas for the same registry reason.

The ledger README states the argument for closing this gap almost verbatim: "A property that is parsed but has no runtime consumer is a silent no-op." And it already documents the hook — SPEC_ONLY_SCHEMAS (scripts/liveness/check-liveness.mts:90), for schemas that are "authorable yet deliberately not registered". webhook is the existing precedent.

Proposed disposition

Not one change — the tiers want different treatment.

  1. search.{fuzzy,operator,boost,minScore,language,highlight} + AggregationNode.filter — mark experimental. Cheapest and safest: an [EXPERIMENTAL — not enforced] .describe() marker, which the ledger already recognizes as a status source. No wire or compat impact. Turns a false affordance into a declaration.
  2. joins, windowFunctions — remove. Both capabilities exist behind a different, working door (expand for joins; SqlDriver.findWithWindowFunctions() for window functions), so removal deletes the second, broken spelling rather than the capability. This is the [P3] data: FieldNode's nested-select object form is declared but nothing produces or consumes it — enforce or remove #4196 argument and Prime Directive Add comprehensive test suite for Zod schema validation #12's.
  3. having — decide, don't default to remove. The strongest enforce candidate of the set: a genuinely expected SQL capability, and the in-memory aggregation fallback could apply it in a few lines. Enforcing also needs aggregate() to stop dropping it (finding 1). Worth a judgment call rather than a unilateral deletion.
  4. cursor, distinct — ADR-0087 conversion/tombstone treatment. These have shipped public producers, so removal must also remove QueryBuilder.cursor() / .distinct(), and distinct's count-suppression side effect (finding 2) has to be either preserved deliberately or dropped deliberately — it is an observable REST change either way.
  5. Close the gate. Fold QueryAST into the liveness ledger through SPEC_ONLY_SCHEMAS so this class stops being invisible. This is the part that keeps the next one from needing a person to notice it.

Steps 1-2 are independent and mechanical. 3 and 4 want a maintainer decision first. 5 is the one worth doing regardless of how 1-4 land.

Note on scope

#4196/#4268 did not create any of this; it made the method visible. The docs callouts were already accurate before this sweep — what is new here is that inertness was never dispositioned, that two of the members have public SDK producers, that one of them is mis-wired rather than dead, and that the gate which would have caught all of it does not reach the request surface.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions