Skip to content

Commit 78bc4ad

Browse files
os-muskclaudeos-support-ai
authored
fix(driver-sql)!: findWithWindowFunctions presents its rows like every other read door (#16609) (#16716)
* fix(driver-sql)!: present findWithWindowFunctions rows like every other read door (#16609) The one record read door that returned `await builder` with no presentation: no `formatOutput` (every `find()`/`findOne()` row gets it) and no `presentReadValue` (`aggregate()`/`distinct()` got it under #3797/#3849). So a declared `Field.boolean` answered `1` where `find()` answered `true`, and a declared `Field.object` answered the stored JSON text where `find()` answered the parsed object. Each row now runs through the same `formatOutput` pass, minus the window function alias columns, which are computed values rather than declared fields. The collision case is ruled and pinned: an alias spelled the same as a declared field already won the key in SQL (`select *` plus `<window> as ok` keeps the last column), and its value now stays raw rather than being folded through the declared type's rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * test(driver-sql): live-dialect arm for the window door, and the full FROM/TO table Contract-review patch round on PR #16716 (findings F2, F3, F5 and the non-governed half of F4). No production code changes. F5 — merged origin/main, so this branch now carries #16619: `formatOutput`'s instant gates are unconditional, which is the presenter this door actually ships through. Every CI leg on the previous head measured the pre-B1 presenter. F3 — `sql-driver-window-function-output.test.ts` gains a `measure(cell)` arm over `DIALECT_CELLS`, declared through `declareDialectCell` so an unprovisioned cell is a NAMED SKIP and never a silent pass. It asserts the two halves the SQLite-only arm cannot: `typeof row.ok === 'boolean'` (the MySQL half of the `isSqlite || isMysql` boolean gate) and the canonical `YYYY-MM-DDTHH:MM:SS.sssZ` text for `closed_at` / `created_at` / `updated_at` (the PG + MySQL instant fold). SS4 reads the same row back through raw knex to prove the fold is the driver's and not the client's. F2 — the changeset gains a per-class, per-dialect FROM/TO table covering all seven classes this door moves: adds `external.columnMap` (remote column key -> local field key, every dialect), the SQLite numeric-string -> `number` move, the MySQL `Field.date` `Date` -> `YYYY-MM-DD` move and `Field.time` -> canonical `HH:MM:SS[.fff]`, and spells the instant TO as the canonical text on every dialect. `minor`, the BREAKING banner and the ADR-0087 disposition are unchanged. F4 (non-governed half) — the header comment of `sql-driver-13973-canonical-iso-read-door.test.ts` said this door applies no read presentation. It routes through `formatOutput` since #16609, so the comment now says that and flags that ADR-0053 D-F1 still records it as not covered, with governed docs-only card #16782 carrying the amendment. `docs/adr/**` is untouched here. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TezFG8ZMrNH6n5VTNpPpdH * docs(changeset): reword the FROM→TO heading so the ADR-0087 gate reads no prescription (#16609) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TezFG8ZMrNH6n5VTNpPpdH --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: os-musk <support@objectstack.ai>
1 parent f315144 commit 78bc4ad

4 files changed

Lines changed: 530 additions & 4 deletions

File tree

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
---
2+
"@objectstack/driver-sql": minor
3+
---
4+
5+
fix(driver-sql)!: `findWithWindowFunctions()` presents its rows like every other read door — a declared boolean answers `true`, not `1` (#16609)
6+
7+
<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable moves: no spec key, export, config field or stored metadata changes spelling or shape, `packages/spec` is untouched, no DDL and no stored bytes change, and `objectstack migrate meta` has nothing to rewrite. What moves is the PRESENTATION one driver door applies to values it reads back — `findWithWindowFunctions()` now runs each row through the same `formatOutput` pass `find()` has always run — so the door stops disagreeing with the driver's own declared read contract. The consumer note below is guidance for code that had compensated for the old storage forms; it prescribes no rewrite of any authored artifact. -->
8+
9+
**BREAKING** on the rows returned by `SqlDriver#findWithWindowFunctions()`.
10+
Shipped as `minor` under the repo's launch-window convention for breaking
11+
changes, matching #3849 — the `aggregate()` / `distinct()` half of this same
12+
gap, which graded `minor` for the same boolean-shape move.
13+
14+
**What was wrong.** `findWithWindowFunctions()` was the one record read door
15+
that returned `await builder` with no presentation at all: no `formatOutput`
16+
(which every `find()` / `findOne()` row gets) and no `presentReadValue` (which
17+
`aggregate()` / `distinct()` got under #3797 / #3849). So it handed back
18+
STORAGE forms where every other door hands back the declared type's
19+
presentation. Measured on SQLite against the built package, one row through the
20+
two doors:
21+
22+
```
23+
find(): { ok: true, closed_at: '2026-01-10T09:00:00.123Z', meta: { k: 1 } }
24+
findWithWindowFunctions(): { ok: 1, closed_at: '2026-01-10T09:00:00.123Z', meta: '{"k":1}', rn: 1 }
25+
```
26+
27+
A declared `Field.boolean` answered `1` where `find()` answered `true`; a
28+
declared `Field.object` answered the stored JSON TEXT where `find()` answered
29+
the parsed object. On Postgres and MySQL the same door handed out the client
30+
library's `Date` for `Field.datetime` and the audit stamps — the one shape every
31+
other read door no longer produces — so on the live dialects the divergence was
32+
between this door and the driver's own declared read contract, not merely
33+
between dialects.
34+
35+
**What moves, per column class and per dialect (storage form → presented form).** Routing this door
36+
through `formatOutput` moves SEVEN classes, not only the boolean and JSON ones
37+
the defect was reported as. `unchanged` means the storage form on that dialect
38+
already WAS the presented form, so the row is byte-identical there — it is
39+
recorded rather than omitted, because the same code path now runs for it.
40+
41+
| class | sqlite | postgres | mysql |
42+
|---|---|---|---|
43+
| `Field.boolean` | `1` / `0` → `true` / `false` | unchanged (native `boolean`) | `1` / `0` → `true` / `false` |
44+
| `Field.object` (JSON) | `'{"k":1}'` TEXT → `{ k: 1 }` | unchanged (native `jsonb`) | unchanged (mysql2 parses JSON) |
45+
| numeric fields | `'4'` → `4` (a numeric STRING off a legacy TEXT-affinity column) | unchanged | unchanged |
46+
| `Field.datetime` + `created_at` / `updated_at` | unchanged — already the canonical text since #3912; a legacy zone-naive row is repaired to it | `Date` → `'2026-01-10T09:00:00.123Z'` | `Date` → `'2026-01-10T09:00:00.123Z'` |
47+
| `Field.date` | unchanged (`toDateOnly` on text is identity) | unchanged (the driver pins the `date` OID parser to text) | `Date` → `'2026-01-10'` |
48+
| `Field.time` | unchanged | `'09:30:00.5'` → `'09:30:00.500'` | → canonical `HH:MM:SS[.fff]` |
49+
| `external.columnMap` | the row KEY renames: remote column key → local field key | same | same |
50+
51+
The instant TO is the canonical `YYYY-MM-DDTHH:MM:SS.sssZ` TEXT **on every
52+
dialect**, never a JS `Date` — that is ADR-0053 D-F1 as #16619 landed it, and
53+
this door now runs the same presenter, so it answers the same shape the other
54+
read doors do.
55+
56+
`external.columnMap` is the one class nobody named on the card, and it is a KEY
57+
move rather than a value move: on an external object with a `columnMap`, the row
58+
this door returns is now keyed by the LOCAL field names, as `find()` has always
59+
keyed it, instead of by the remote physical column names.
60+
61+
**What to do.** Code that compensated for the storage forms stops being
62+
correct and should simply drop the compensation:
63+
64+
- `if (row.ok === 1)` → `if (row.ok)`; the value is a real boolean now.
65+
- `JSON.parse(row.meta)` → `row.meta`; it is already the parsed value, and
66+
parsing an object throws.
67+
- `Number(row.amount)` → `row.amount`; a numeric column is a `number`.
68+
- A `Field.datetime` / `Field.date` / `Field.time` / `created_at` / `updated_at`
69+
read through this door is now the same presented value `find()` gives, so a
70+
branch that re-normalised it — or that called `Date` methods on it — can go.
71+
- A reader of an external object with a `columnMap` indexes the row by the LOCAL
72+
field key, not the remote column key.
73+
74+
**The alias columns are carved out**, which is the design question this door
75+
raised. A window alias is a computed value, not a declared field, so no declared
76+
field's presentation rule touches it. When an alias is spelled the same as a
77+
declared field, SQL had already decided which value wins the key — `select *`
78+
plus `<window> as ok` projects two columns named `ok` and the row keeps the
79+
LAST, so the computed value wins and the declared column's value is not in the
80+
row at all. That is unchanged. What is now ruled is that the winning value stays
81+
RAW: presenting a `row_number` of `1` and `2` as the declared boolean would fold
82+
both to `true` and destroy the value the caller asked for. This is the same
83+
ruling `aggregate()` already makes for a date-bucketed column aliased as its own
84+
field name.

‎packages/drivers/driver-sql/src/sql-driver-13973-canonical-iso-read-door.test.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,15 @@
1717
* an Invalid `Date`, which has no canonical text and passes through as the
1818
* `Date` it is (pinned by `sql-driver-14078-invalid-date-materialisation.test.ts`;
1919
* never met here, because this fixture writes only valid instants).
20-
* `findWithWindowFunctions` is not one of those doors: it applies no read
21-
* presentation of any kind, D-F1 records it as not covered, and #16609 holds
22-
* it.
20+
* `findWithWindowFunctions` used to be the one read door outside this list: it
21+
* applied no read presentation of any kind. Since #16609 it routes each row
22+
* through the SAME `formatOutput` pass `find()` runs (minus the window-alias
23+
* columns), so it presents these two column classes exactly as the doors above
24+
* do — pinned by `sql-driver-window-function-output.test.ts`, whose live cells
25+
* assert the canonical instant on Postgres and MySQL for this door too.
26+
* ⚠️ ADR-0053 D-F1 still RECORDS that door as not covered; the tree is ahead of
27+
* the declaration there, and docs-only governed card #16782 carries the
28+
* amendment. Do not read the ADR line as the current behaviour.
2329
*
2430
* §A1–§A3 measure the four row doors on the fixture table; §A5–§A7 the three
2531
* write doors whose return is a row, on a second table so their writes cannot

0 commit comments

Comments
 (0)