Skip to content

fix(driver-sql): aggregate() attributes an unresolvable column to its clause - #11729

Merged
huangyiirene merged 2 commits into
mainfrom
claude/issue-11541-aggregate-unresolvable-column-refusal
Aug 24, 2026
Merged

fix(driver-sql): aggregate() attributes an unresolvable column to its clause#11729
huangyiirene merged 2 commits into
mainfrom
claude/issue-11541-aggregate-unresolvable-column-refusal

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Fixes#11541

What changed — contract surface, named (clause ②)

SqlDriver.aggregate() stops answering an unresolvable column with the generic terminal and instead answers the same class of refusal find() and count() give, attributed to the clause the caller's own query names the column in:

condition on the aggregate doorbefore (main @ 4e786cd)after
unresolvable column named by the whereDATABASE_ERROR / 500INVALID_FILTER / 400 — #8790's unresolvableFilterColumnRefusal, byte-identical message to find()/count()
unresolvable column named by a groupBy field or an aggregation fieldDATABASE_ERROR / 500INVALID_FIELD / 400, message naming the clause and the column, field + object set the way the ingress door's INVALID_FIELD family carries them
recognised unresolvable-column class whose wording parses to no nameDATABASE_ERROR / 500unchanged — the #11455 terminal envelope; with no name there is no AST lookup, so no attribution is supportable (#8931)
everything else (missing table, 42883, syntax, connection)DATABASE_ERROR / 500unchanged

New message (groupBy arm, aggregation arm analogous): The groupBy of this query names 'nosuchcol', a column that object 'x' has no column for, so the aggregate never ran. … Check the name against the object's fields; if the field was declared recently, run schema sync so the column exists before aggregating on it. The dialect text (statement + inlined bound literals) goes to the server log only, same as the #8790 WHERE refusal.

Implementation: one new classifier SqlDriver.aggregateBackendFault(object, query, error) called from aggregate()'s catch, plus the composer unresolvableAggregateColumnRefusal. isUnresolvableColumnError / unresolvableColumnNameOf are untouched (no new dialect recognizer; MySQL's clause-position wording is deliberately unread — the query AST decides). Subclasses (driver-turso embedded face, driver-sqlite-wasm) inherit.

Why INVALID_FIELD and not INVALID_QUERY — read off the repo, not preferred

The dispatch flagged this choice for verification. Evidence, in order of weight:

  1. Exactly-on-point precedent: the protocol ingress already refuses this very condition — a grouping/aggregation target the object does not have — with 400 INVALID_FIELD + field + object (assertGroupByFieldsExist / assertAggregationFieldsExist, packages/metadata-protocol/src/protocol.ts, REST 读路径:searchFields / groupBy / aggregations 指向不存在的字段时被静默降级(#4226 收口后剩下的三条轴) #4254), reserving INVALID_QUERY on those axes for entries the spec cannot read (shape violations). This condition is a well-shaped query naming a missing column, i.e. the first kind.
  2. The two-layer rule this file already follows: driver-sql: one unresolvable WHERE column, two answers — find() silently returns [] while count() throws a raw dialect error with no ADR-0112 envelope #8790's WHERE refusal took the ingress door's code (INVALID_FILTER) for the same condition one layer down, on the stated principle that one condition refused at two layers must not be explained two ways. Applied to the groupBy/aggregations axes, that yields INVALID_FIELD.
  3. drivers(sql,turso): Unsupported aggregate function 两面都是裸 Error(code/status 皆 undefined),且三个 spec 已声明的聚合函数无任何 SQL 后端编译 #5907's INVALID_QUERY is a different condition: a function name the protocol does not declare — a query no backend can run. A missing column is a query that answers rows the moment schema sync runs. (The card's own honest bound said the drivers(sql,turso): Unsupported aggregate function 两面都是裸 Error(code/status 皆 undefined),且三个 spec 已声明的聚合函数无任何 SQL 后端编译 #5907 precedent covers function names, not columns.)
  4. No code is minted (ADR-0112 D3/D4): INVALID_FIELD is a standard-catalog member and already the write path's answer for an unknown column at the REST boundary; @objectstack/rest's existing INVALID_FIELD branch serves the enriched envelope unchanged.

Five-row probe, re-measured on all three dialects

Before (main @ 4e786cd, live PG 16 + MySQL 8.0.46 + embedded SQLite — identical on all three):

find where nosuchcol => INVALID_FILTER 400
count where nosuchcol => INVALID_FILTER 400
agg where nosuchcol => DATABASE_ERROR 500
agg avg(nosuchcol) => DATABASE_ERROR 500
agg groupBy nosuchcol => DATABASE_ERROR 500

After (this branch, same three dialects — identical on all three):

find where nosuchcol => INVALID_FILTER 400 "Filter on 'nosuchcol' names a column that object … has no column for…"
count where nosuchcol => INVALID_FILTER 400 (identical message)
agg where nosuchcol => INVALID_FILTER 400 (identical message — pinned byte-identical to find()'s)
agg avg(nosuchcol) => INVALID_FIELD 400 field=nosuchcol "An aggregation of this query names 'nosuchcol'…"
agg groupBy nosuchcol => INVALID_FIELD 400 field=nosuchcol "The groupBy of this query names 'nosuchcol'…"

Tests

New suite sql-driver-11541-aggregate-unresolvable-column-refusal.test.ts: the five-row parity pins per dialect cell (SQLite always; live PG + MySQL cells ran here), attribution pins (structured groupBy entry, both-clause case, where+groupBy same column), disclosure pins (no statement shape, no bound literal, dialect text to server log), controls (working aggregate, honest empty answer, missing table keeps the terminal), and a classifier block on crafted wordings covering all three arms and the fences (exact-match — a dotted title.x is never attributed to an aggregation over x; MySQL's clause position unread; an alias never treated as a column).

  • Full @objectstack/driver-sql suite at CI parity (TZ=America/New_York, PG at Asia/Shanghai, MySQL at +08:00, live cells running): 130 files / 2651 tests passed at a49b099349 (clean tree), vitest exit read pre-pipe via the verify-lock entry point (VERDICT command-exit 0).
  • pnpm --filter @objectstack/driver-sql typecheck green at a49b099349; driver-turso and driver-sqlite-wasm typecheck green (turbo run, 17 successful).
  • Reverse verification (ablation): attribution arms disabled in aggregateBackendFault behind an ABLATION-11541 marker; mutation proven on disk (marker grep count 1), run: 12 failed / 16 passed / 5 skipped — every red inside the new suite (attribution pins receiving DATABASE_ERROR), while sql-driver-11455-aggregate-fault-envelope.test.ts stayed fully green; restore proven (marker count 0, git checkout HEAD), post-restore full suite green (the 130/2651 run above). The suite imports ./sql-driver.js relatively, so vitest compiles the mutated source directly — no dist build in the ablation loop.

Gates (all at a49b099349)

Derived by node scripts/pm/dispatch-gates.mjs (no paths passed; stderr names this repo/commit). All exit 0, verdict lines read from each gate's own output: check:changeset-gate-self-tests, check:objectui-changeset, check:published-files, check:slot-lookup, check:test-source-alias, check:type-source-resolution, check:query-options-erasure, check:type-check-coverage, check:engine-double-contract (399 pinned / 133 DEBT / 2 exempt), check:cross-package-test-inputs, check:where-matcher, check:nul-bytes, check:error-code-casing, check:driver-conformance, check-adr-0087-registration (non-breaking changeset, no marker owed), check-changeset-no-major, check-empty-changeset, check-plugin-teardown-shape, docs-audit/check-affected-docs, release-rehearsal-clone --self-test.

Declared narrowing: check:type-check-debt not run locally (its re-measure requires the full workspace closure built). Evidence the narrowing excludes nothing: the diff's only package is driver-sql, which appears in neither the DEBT nor the TEST_DEBT ledger of scripts/check-type-check-coverage.mjs (grep over both ledger spans: 0 hits); its own tsc --noEmit includes the new test files (it went red on an interim test-file error, then green) — and the diff edits no tsconfig/turbo/ledger, so no other package's tsc program changes. CI runs the full gate.

Driver-conformance lane numbers: before 45 covered / 0 DEBT / 0 exempt → after 45 covered / 0 DEBT / 0 exempt (the matrix counts case-set coverage; this card adds refusal pins, not case-set rows).

Scope notes


Generated by Claude Code

…ause
aggregate() answered DATABASE_ERROR/500 for an unresolvable column where
find() and count() answer the #8790 refusal. The dialect-named column is
now attributed to the clause the caller's own query names it in: a
groupBy field or an aggregation field refuses INVALID_FIELD/400 (the
ingress door's own code for this condition), a column in neither clause
is the WHERE and keeps the #8790 INVALID_FILTER/400 refusal verbatim,
and a wording that parses to no name keeps the #11455 terminal envelope
unchanged (no supportable attribution, per the #8931 ruling).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VK8rFDtg8eREaxBGX99Csn
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/driver-sql, touching 3 documentable anchor(s).

6 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/drivers.mdx(via SqlDriver (symbol))
  • content/docs/data-modeling/index.mdx(via SqlDriver (symbol))
  • content/docs/plugins/packages.mdx(via SqlDriver (symbol))
  • content/docs/protocol/kernel/index.mdx(via SqlDriver (symbol))
  • content/docs/protocol/kernel/lifecycle.mdx(via SqlDriver (symbol))
  • content/docs/protocol/objectql/query-syntax.mdx(via SqlDriver (symbol))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx(via SqlDriver (symbol))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json e170b0ae5399c98097522ac1224e8101b867c22bpackageMentionDocs.

Which tree this was computed on

This run read content/docs from a32683859464dd8b5a5e202834c3339a5e3c491e — the merge of head a49b099349d78765ce079f7a88a90c1e14954d47 into base e170b0ae5399c98097522ac1224e8101b867c22b, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin a32683859464dd8b5a5e202834c3339a5e3c491e && git checkout a32683859464dd8b5a5e202834c3339a5e3c491e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e170b0ae5399c98097522ac1224e8101b867c22b a49b099349d78765ce079f7a88a90c1e14954d47 && git checkout -B drift-repro e170b0ae5399c98097522ac1224e8101b867c22b && git merge --no-ff a49b099349d78765ce079f7a88a90c1e14954d47
node scripts/docs-audit/affected-docs.mjs --json e170b0ae5399c98097522ac1224e8101b867c22b

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs e170b0ae5399c98097522ac1224e8101b867c22b → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 24, 2026
@huangyiirene
huangyiirene marked this pull request as ready for review August 24, 2026 14:37
@huangyiirene
huangyiirene added this pull request to the merge queueAug 24, 2026
Merged via the queue into main with commit ef52884Aug 24, 2026
32 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-11541-aggregate-unresolvable-column-refusal branch August 24, 2026 14:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@huangyiirene@claude