Skip to content

Commit 9ccc417

Browse files
os-muskclaude
andauthored
fix(driver-sql): attribute an unresolvable distinct() column to the clause the caller named (#17945)
Fixes #17857 Clause-②: no — the call is refused either way; what changes is the refusal's envelope and attribution. No input that was previously refused becomes accepted, and no exported symbol moves. `distinct()` was the last read door answering a **server fault** for one unresolvable column. This lands the attribution arm #17639 deliberately left filed, in the shape #11541 established for `aggregate()`. ## The five rows, run rather than reasoned about Embedded SQLite, one object with a single `title` column, one column name the table does not have. Before = `origin/main` at `dbea1756d9`; after = this branch at `15f26e6210`, where the full package suite, the repo lint and the derived gate set were all re-run. | door | before | after | arm | |:--|:--|:--|:--| | `count(t, { where: { nosuchcol: 1 } })` | `INVALID_FILTER` / 400 | unchanged | (not this door) | | `find(t, { where: { nosuchcol: 1 } })` | `INVALID_FILTER` / 400 | unchanged | (not this door) | | `aggregate(t, { groupBy: ['nosuchcol'] })` | `INVALID_FIELD` / 400 | unchanged | (not this door) | | `distinct(t, 'title', { nosuchcol: 1 })` | **`DATABASE_ERROR` / 500** | **`INVALID_FILTER` / 400** | **arm 2** (a predicate named it) | | `distinct(t, 'nosuchcol')` | **`DATABASE_ERROR` / 500** | **`INVALID_FIELD` / 400** | **arm 1** (the listed field) | After-row 4 is **byte-identical** to what `find()` and `count()` answer for the same filter — asserted as message equality, not merely as the same code, because a drifted wording is how one condition acquires two explanations. After-row 5 names the listed column and the word *filter* appears nowhere in it. ## The three arms `SqlDriver.distinctBackendFault` reads the dialect-named column back against the caller's own request. The dialect names the column; it does not name the clause. 1. the name **equals the `field` argument** ⇒ `INVALID_FIELD` / 400 naming the listed column, carrying the `field` and `object` riders the ingress door's refusals carry; 2. it does not ⇒ the statement's only remaining column sources are the WHERE compiled from `filters` and the tenant-scope predicate, both filters, so #8790's existing `unresolvableFilterColumnRefusal` applies verbatim; 3. the wording yields **no name** ⇒ no attribution is supportable and #17639's terminal `DATABASE_ERROR` / 500 stands unchanged. No blanket `isUnresolvableColumnError` arm: the classifier is what makes that claim unnecessary, not what makes it safe. No code is minted and no new dialect recognizer is added — both predicates are the ones `find()`, `count()` and `aggregate()` already share. ### One deliberate departure from the dispatch brief, with the measurement behind it The brief paraphrased arm 2 as *"the name appears in the `filters` AST"* with *"neither"* falling to the terminal. The card's own text is the #11541 mirror — arm 2 is the **complement** of arm 1 — and that is what landed, because the two designs are distinguishable and one of them re-opens the defect. See ablation leg C below. Both designs produce the identical five rows above, so the divergence is invisible to the card's own probe. ## Ablation — three legs, each with on-disk proof and a verified restore The fix was committed first; every leg mutated the committed file, proved the mutation reached disk by marker count and by `git hash-object` differing from the HEAD blob, ran the suite, restored with `git checkout HEAD -- PATH`, and proved the restore by the on-disk blob hash equalling the HEAD blob again with `git diff HEAD` empty. | leg | mutation | result | |:--|:--|:--| | A | delete arm 1 (i.e. the forbidden blanket arm) | **6 failed** / 12 passed — every arm-1 case, the five-row case and the tie-break | | B | delete arm 2 (every non-field column falls to the terminal) | **6 failed** / 12 passed — every arm-2 case, both dialect-wording units and the five-row case | | C | replace the complement with a top-level `filters` key search | **3 failed** / 15 passed | Leg C is the discriminating one. The **flat**-filter case (`arm 2: a filter column gets byte-identical treatment at all three doors`) stayed **green**, while the **nested** case failed: ``` AssertionError: $or: code: expected 'DATABASE_ERROR' to be 'INVALID_FILTER' ``` `FilterCondition` nests, so an AST search silently downgrades `{ $or: [{ nosuchcol: 1 }] }` back to the 500 this card removes — the defect itself, one nesting level down, invisible to any pin written on a flat filter. The complement needs no walker to be complete: the compiled statement is `select distinct FIELD from TABLE where FILTERS-PLUS-TENANT-SCOPE`, so a column reference that is not the `field` came from a predicate. Marker hashes, for the record: HEAD blob `6b7457d056`; leg A `18c0dcaf80`, leg B `ee90517fce`, leg C `abcb2146c3`; all three restored to `6b7457d056` with a residual marker count of 0. ## Changeset — measured, not assumed: `patch` The expectation was `patch`; the measurement confirms it rather than being taken from the sibling's precedent. `@objectstack/driver-sql` declares `files: ["dist","README.md","CHANGELOG.md"]`, so `dist/` was built and grepped with both controls: | reading | result | |:--|:--| | positive control `aggregateBackendFault` (published since #11541) | hits `dist/index.js`, `dist/index.mjs`, `dist/index.d.ts`, `dist/index.d.mts` | | this change's marker `distinctBackendFault` | hits the **same four** published files | | this change's refusal sentence `lists the distinct values of` | hits `dist/index.js`, `dist/index.mjs` | | fabricated negative control `distinctBackendFaultNeverShipped` | **0** | | test-only symbol `ClassifierProbe` | **0** | A `protected` member does reach the published declaration: ``` 3819: protected distinctBackendFault(object: string, field: string, error: unknown): Error; 3860: protected unresolvableDistinctColumnRefusal(object: string, column: string, error: unknown): Error; ``` Published bytes move ⇒ `patch`, never `skip-changeset`. ## Verification - `pnpm --filter @objectstack/driver-sql test` — **exit 0**, 177 files passed / 11 skipped, 2620 tests passed / 166 skipped. The skips are the live PG and MySQL cells, unprovisioned here and covered by the `Temporal Conformance (live PG + MySQL)` job. - `pnpm --filter @objectstack/driver-sql typecheck` — **exit 0**. - `pnpm --filter '@objectstack/driver-sql^...' build` — **exit 0** (dependency closure). - `pnpm lint` (repo-wide `eslint . --no-inline-config`) — **exit 0**, clean, no narrowing declared. - Gate families derived from the diff with `scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`: **62 derived, 62 run, 59 exit 0**, re-run in full at the final HEAD. The three non-zero are all `PREREQUISITE NOT MET` / **exit 3 — NOT MEASURED, neither pass nor failure** — because they read built output for the whole workspace: `check:dual-build-cjs-loads`, `check:lean-entry-closure`, `check:type-check-debt`. CI builds the workspace before those steps. - `check:doc-authoring` caught a real defect in this diff's first draft — the new `logger.warn` string carried `(#7929, #17857)`, and a log line reaches operators and generated surfaces where an issue id resolves against nothing. The ids moved to the adjacent comment; the gate is green. ## Acceptance notes - **Not filed, noted here.** `driver-memory`'s half of the bare-filter asymmetry — a bare filter there returns the *unfiltered* set in silence — is still open and is already recorded in `SqlDriver.distinct`'s own docblock (the #5499 freeze dissolved 2026-08-11 and nothing has claimed it since). Untouched by this card, which is about a column that does not exist rather than a filter that is silently dropped. Carrier: whoever takes the `driver-memory` filter-narrowing parity work; the note sits in the docblock a reader of this door already passes through. - **Fence held.** #17590 (whether a `json` column should *answer* a distinct read) is not addressed here and remains open: that is a column that **exists** whose type has no equality operator, so it is not an unresolvable-column error and still leaves as #17639's terminal. `sql-driver-17639-distinct-fault-envelope.test.ts` is unchanged except for its head note, which said the `distinct()` attribution half "is filed as its own card" — made false by this PR and corrected in the same diff. --- _Generated by [Claude Code](https://claude.ai/code/session_01RuoNSXUbBoWHkNS4AknTrM)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2c87a48 commit 9ccc417

4 files changed

Lines changed: 650 additions & 16 deletions

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
"@objectstack/driver-sql": patch
3+
---
4+
5+
`SqlDriver.distinct()` now answers **one unresolvable column the way the other three read doors do** — a `400` that names it — instead of a `DATABASE_ERROR` / `500` server fault.
6+
7+
The same condition (a column name the table does not have) asked at four doors used to get three answers and one server fault. Measured on `origin/main` at `dbea1756d9`, embedded SQLite, and identical on live PostgreSQL 16.13:
8+
9+
| door | before | after |
10+
|:--|:--|:--|
11+
| `count(t, { where: { nosuchcol: 1 } })` | `INVALID_FILTER` / 400 | unchanged |
12+
| `find(t, { where: { nosuchcol: 1 } })` | `INVALID_FILTER` / 400 | unchanged |
13+
| `aggregate(t, { groupBy: ['nosuchcol'] })` | `INVALID_FIELD` / 400 | unchanged |
14+
| `distinct(t, 'title', { nosuchcol: 1 })` | **`DATABASE_ERROR` / 500** | **`INVALID_FILTER` / 400** |
15+
| `distinct(t, 'nosuchcol')` | **`DATABASE_ERROR` / 500** | **`INVALID_FIELD` / 400** |
16+
17+
A caller's own mistake — a field name that does not exist — was served as a server fault naming nothing they could act on, one door away from a `400` that names the column. A picklist-populating `distinct()` sits beside the `find()` and `count()` of the same list view.
18+
19+
**Attribution comes from the caller's own request, never from the backend's prose.** The dialect names the column but not the clause, so the clause is read off the call this driver just compiled — the shape `aggregateBackendFault` established for `aggregate()`:
20+
21+
1. the name **equals the `field` argument** ⇒ `INVALID_FIELD` / 400 naming the listed column, with the `field` and `object` riders the ingress door's refusals carry;
22+
2. it does not ⇒ the statement's only remaining column sources are the WHERE compiled from `filters` and the tenant-scope predicate, both filters, so the existing `INVALID_FILTER` refusal applies verbatim — the same sentence `find()` and `count()` give;
23+
3. the dialect wording yields **no name** ⇒ no attribution is supportable and the terminal `DATABASE_ERROR` / 500 envelope stands unchanged.
24+
25+
Arm 2 is the **complement** of arm 1 rather than a search of the `filters` AST, which keeps a nested filter (`{ $or: [{ nosuchcol: 1 }] }`) on the same `400` as a flat one.
26+
27+
⛔ **No input that was refused before is accepted now, and no exported symbol moves.** The call fails either way; what changes is the refusal's code, status and words. No error code is minted — `INVALID_FIELD` is a standard-catalog member (ADR-0112) and already this repo's answer for a named column an object does not have. No new dialect recognizer is added: both predicates are the ones `find()`, `count()` and `aggregate()` already share.
28+
29+
A caller that branched on `DATABASE_ERROR` / `500` for a mistyped `distinct()` field or filter key now sees `INVALID_FIELD` / `INVALID_FILTER` `400`s instead; that is the point of the change, and it matches what the same mistake already returned from every other read door.

‎packages/drivers/driver-sql/src/sql-driver-17639-distinct-fault-envelope.test.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,12 @@
7575
* compiled from `filters`. A blanket arm would tell the author of
7676
* `distinct(o, 'nosuchcol')` — who passed no filter at all — that their FILTER
7777
* was wrong. #11541 closed that gap for `aggregate()` with a clause-attributing
78-
* classifier; the `distinct()` half is filed as its own card. What this suite
79-
* pins is unchanged by that: an error the classifier does NOT claim still
80-
* leaves as this terminal envelope.
78+
* classifier; #17857 has since closed the `distinct()` half the same way
79+
* (`SqlDriver.distinctBackendFault`, pinned by
80+
* `sql-driver-17857-distinct-unresolvable-column-refusal.test.ts`). What this
81+
* suite pins was unchanged by that, exactly as this note anticipated: an error
82+
* the classifier does NOT claim — a table that was never provisioned, a `json`
83+
* column with no equality operator — still leaves as this terminal envelope.
8184
*/
8285

8386
import { describe, it, expect, beforeAll, afterAll } from 'vitest';

0 commit comments

Comments
 (0)