diff --git a/.changeset/numeric-column-representation.md b/.changeset/numeric-column-representation.md new file mode 100644 index 0000000000..5bc600b56c --- /dev/null +++ b/.changeset/numeric-column-representation.md @@ -0,0 +1,70 @@ +--- +'@objectstack/spec': minor +'@objectstack/driver-sql': minor +'@objectstack/cli': minor +--- + +One physical representation for the NUMERIC column family, read by every producer of DDL + +`packages/spec` now states, per field type, what column a numeric field gets, and all three +producers read it: `SqlDriver.createColumn`, `os generate migration --format sql` and +`os generate migration --format typescript`. Measured on live PostgreSQL 16.13, one object +through all three producers, before and after: + +``` + BEFORE AFTER + driver sql gen ts gen all three +number real numeric(18,2) numeric(8,2) numeric(65,30) +currency real numeric(18,2) numeric(8,2) numeric(65,30) +percent real numeric(5,2) numeric(8,2) numeric(65,30) +slider real numeric(18,2) numeric(8,2) numeric(65,30) +summary real numeric(18,2) numeric(8,2) numeric(65,30) +progress real numeric(5,2) numeric(8,2) numeric(65,30) +rating real integer integer integer +``` + +7 of 7 columns diverged before, 0 of 7 after. Every arm of the old split lost data in its own +direction: `real` is IEEE-754 binary32, so a `currency` of `1234567.89` read back `1234567.9`; +`numeric(5,2)` and `numeric(18,2)` silently ROUND a legitimate `33.333` to `33.33` (round +half-up — executed, not inferred); `numeric(8,2)` refused `1234567.89` outright. `65,30` is +MySQL's documented `DECIMAL` maximum and therefore the portable one, and it is the only +candidate measured to lose nothing on a nine-value corpus. + +Both migration formats also take the physical `NOT NULL` from `storage.notNull` and never from +`required`, which is where `SqlDriver.createColumn` has taken it since ADR-0113: `required` is +the write-time contract the record validator enforces, and binding the DDL to it made every +post-deploy tightening a destructive migration. + +**BREAKING** — new columns only; no existing column is retyped, no migration is planned, and no +backfill runs. Three consequences to know before creating new tables: + +- `rating` is an INTEGER column, and the two server dialects dispose of a fractional star count + DIFFERENTLY — do not read one answer for both. PostgreSQL REFUSES `4.5` outright, where a + `real` column accepted it. MySQL does NOT refuse: it ROUNDS, and `4.5` becomes `5` with no + error, which is a silent alteration and the reason to declare a `slider` (in the exact-decimal + set) for anything that wants fractional values. SQLite refuses nothing either: it stores `4.5` + as a REAL in an INTEGER-affinity column, unchanged from today. +- An exact-decimal column is bounded where a float is not, in BOTH directions. It keeps 30 + fractional digits: a magnitude whose significant digits run past the 30th decimal place loses + the tail silently — `1.2345678901234567e-15` stores as `0.000000000000001234567890123457`, so + the loss begins around |x| < 1e-13 and is total below 1e-30 — and magnitudes at or above 1e35 + are REFUSED, where `real` kept about seven significant digits out to ~1e38. A refusal is loud; + the rounding it replaces was not. +- Reads are bounded by the wire contract, not by the column. `find()` hands back a JS number + (`z.number().finite()`), so a value that was never a JS double does not survive the round trip + exactly — `1234567890123456.123` reads back `1234567890123456`, and 2^53+1 reads back 2^53. + The fidelity this buys is an exact COLUMN read through a double: values written by this + platform round-trip exactly, and SQL-side writers, `summary` roll-ups computed in SQL and any + magnitude at or above 2^53 are bounded by the read seam. Widening that is a wire-contract + change and is not in this release. +- A generated migration no longer emits `NOT NULL` for a field marked only `required: true`. + Declare `storage: { notNull: true }` for a physical constraint — which is what the platform's + own table has always done since ADR-0113, and what `os migrate meta` deliberately does NOT + supply on your behalf (the conversion that stamped it was withdrawn by maintainer ruling on + 2026-09-08). A source author who wants the column they had must write that block themselves; + `required: true` keeps its own meaning, the write-time contract the record validator enforces. + +SQLite emits byte-identical DDL for the six exact-decimal members: knex compiles both +`table.decimal(name, p, s)` and `table.float(name)` to the same `float` column there. + + diff --git a/content/docs/protocol/objectql/types.mdx b/content/docs/protocol/objectql/types.mdx index 1f9ab008ba..3c9855c99d 100644 --- a/content/docs/protocol/objectql/types.mdx +++ b/content/docs/protocol/objectql/types.mdx @@ -263,9 +263,19 @@ quantity: - `min`/`max`: Range validation **Database mapping:** -- SQL driver: a floating-point column (`REAL` on PostgreSQL/SQLite, `FLOAT` on - MySQL). `precision`/`scale` are validation and display metadata — the DDL does - **not** emit `NUMERIC(precision, scale)`. +- SQL driver: an **exact decimal** column at a fixed, portable size — + `NUMERIC(65,30)` on PostgreSQL, `DECIMAL(65,30)` on MySQL, `float` on SQLite + (which applies neither precision nor scale). The size comes from one + per-field-type table in `packages/spec` + (`NUMERIC_COLUMN_REPRESENTATION`), which the driver and both + `os generate migration` formats all read, so one declaration produces one + column whoever builds the table. + ⚠️ The field's own `precision`/`scale` remain validation and display + metadata: the DDL does **not** emit `NUMERIC(precision, scale)` from *your* + declared numbers — the column is the fixed pair above whatever the field + declares. + ⚠️ **New columns only.** Nothing retypes a column that already exists; a + table created before this keeps its `REAL`/`FLOAT` columns and their values. - MongoDB: `Number` **Use cases:** @@ -309,8 +319,17 @@ deprecated in the spec. - `precision` (0–10, default 2) for decimal places **Database mapping:** -- SQL driver: a floating-point column (`REAL` / `FLOAT`) — one column, no - companion currency column and no JSON blob +- SQL driver: the same **exact decimal** column as `number` + (`NUMERIC(65,30)` / `DECIMAL(65,30)`; `float` on SQLite) — one column, no + companion currency column and no JSON blob. Money is the member where the + binary32 `REAL` this replaced lost a correctness question rather than a + display one: `1234567.89` read back `1234567.9` from a `REAL` column. + ⚠️ The column is **not** a blanket `DECIMAL(18,2)`: the platform's own CLDR + table carries 0-digit currencies (JPY, KRW) and 3-digit ones (BHD, KWD), and + the currency-code schema fails open for crypto and custom codes, so a money + column that fixes two decimals is wrong for a set the platform declines to + close. + ⚠️ **New columns only** — see `number` above. - MongoDB: `Number` --- @@ -331,7 +350,11 @@ discount_rate: **Storage:** the percentage **number itself** — `25.5` means 25.5%, matching the `min: 0` / `max: 100` bounds above. It is *not* rescaled to a 0–1 ratio on write. -Physically it is the same floating-point column as `number`. +Physically it is the same **exact decimal** column as `number` +(`NUMERIC(65,30)` / `DECIMAL(65,30)`; `float` on SQLite), on new tables only. +That width is what holds a legitimate `33.333` — the narrow `NUMERIC(5,2)` the +`--format sql` generator used to emit rounded it half-up to `33.33`, and +rounded the 0–1 fraction storage of the same value to `0.33`. The separate `percent` **template filter** (`{{ record.rate | percent }}`) does @@ -1174,7 +1197,8 @@ The column each type gets from the SQL driver, per dialect: |---------------|------------|-------|--------| | `text` / `textarea` / `html` | `TEXT` \* | `TEXT` \* | `TEXT` \* | | `email` / `url` / `phone` / `password` | `VARCHAR(maxLength)` † | `VARCHAR(maxLength)` † | `VARCHAR(maxLength)` † | -| `number` / `currency` / `percent` | `REAL` | `FLOAT` | `REAL` | +| `number` / `currency` / `percent` / `slider` / `progress` | `NUMERIC(65,30)` ‡ | `DECIMAL(65,30)` ‡ | `float` ‡ | +| `rating` | `INTEGER` ‡ | `INT` ‡ | `INTEGER` ‡ | | `date` | `DATE` | `DATE` | `TEXT` (`YYYY-MM-DD`) | | `datetime` | `TIMESTAMPTZ` | `DATETIME(3)` | `TEXT` (canonical `…Z`) | | `time` | `TIME` | `TIME(3)` | `TEXT` (`HH:MM:SS[.fff]`) | @@ -1182,7 +1206,7 @@ The column each type gets from the SQL driver, per dialect: | `select` / `radio` | `VARCHAR(255)` | `VARCHAR(255)` | `VARCHAR(255)` | | `multiselect` / `tags` | `JSON` | `JSON` | `TEXT` (JSON) | | `lookup` / `master_detail` / `tree` | `VARCHAR(255)` | `VARCHAR(255)` | `VARCHAR(255)` | -| `summary` | `REAL` | `FLOAT` | `REAL` | +| `summary` | `NUMERIC(65,30)` ‡ | `DECIMAL(65,30)` ‡ | `float` ‡ | | `autonumber` | `VARCHAR(255)` | `VARCHAR(255)` | `VARCHAR(255)` | | `formula` | *(no column — virtual)* | *(no column)* | *(no column)* | | `json` / `location` / `address` | `JSON` | `JSON` | `TEXT` (JSON) | @@ -1209,6 +1233,22 @@ Note the neighbouring rows that deliberately do **not** follow this rule: runtime-issued number — in none of those is the stored string the value the field's `maxLength` describes, so all of them keep `VARCHAR(255)`. +‡ **The numeric family is NEW COLUMNS ONLY.** The size is one per-field-type +table in `packages/spec` (`NUMERIC_COLUMN_REPRESENTATION`) that +`SqlDriver.createColumn` and both `os generate migration` formats read, so the +three producers no longer disagree; before it they emitted `REAL`, +`NUMERIC(18,2)`/`NUMERIC(5,2)` and `NUMERIC(8,2)` for the same declaration. +`65,30` is MySQL's documented `DECIMAL` maximum and therefore the portable one. +Nothing retypes an existing column, plans a migration, or reports drift over the +difference — a table created before this keeps its `REAL`/`FLOAT` columns, and a +new numeric field added to it gets an exact-decimal column beside them. +On SQLite the exact-decimal members compile to the same `float` column the +driver emitted before (knex's SQLite `decimal` compiler is the literal `float`), +so SQLite keeps REAL affinity and gains no exactness; `rating` moves to INTEGER +affinity there and SQLite still accepts a fractional value as a REAL. The +refusal `rating` gains is a PostgreSQL/MySQL effect: PostgreSQL refuses a +fractional star count outright, and MySQL **rounds** it (4.5 arrives as 5). + Any field flagged `multiple: true` becomes a `JSON` column regardless of its type. Relationship columns are plain id strings with no database `FOREIGN KEY` constraint (see `lookup` above). The MongoDB driver is schemaless — it issues no diff --git a/content/docs/references/api/sortability.mdx b/content/docs/references/api/sortability.mdx index 9550e49365..36b007cd8d 100644 --- a/content/docs/references/api/sortability.mdx +++ b/content/docs/references/api/sortability.mdx @@ -65,10 +65,14 @@ measured degradation is not refused; the projection covers all four: ## Considered and deliberately NOT members - `summary` / `autonumber` — the other two `COMPUTED_VALUE_TYPES`. They sort - CORRECTLY (`summary` is an engine-maintained `table.float`, `autonumber` - an engine-assigned `table.string`; measured on #6924), which is exactly - why virtuality is judged by the storage predicate and never by the write - contract — widening would refuse the two types that work. + CORRECTLY (`summary` is an engine-maintained numeric column — `table.float` + when #6924 measured it, an exact `table.decimal` on new tables since + #16318's stated representation — and `autonumber` an engine-assigned + `table.string`), which is exactly why virtuality is judged by the storage + predicate and never by the write contract — widening would refuse the two + types that work. ⚠️ The column TYPE is not what makes them sortable — + having a PROVISIONED column is — which is why #16318's retype of the + numeric family moved nothing in this projection. - `encrypted` / `secret` / `json` / `vector` and the other heavy or masked types — every one has a stored column, neither door refuses an ORDER BY over one, and the drivers execute it. Marking them unsortable here would diff --git a/packages/cli/src/commands/generate-multiple-json-column.pin.test.ts b/packages/cli/src/commands/generate-multiple-json-column.pin.test.ts index bd78954a1d..060b5403a9 100644 --- a/packages/cli/src/commands/generate-multiple-json-column.pin.test.ts +++ b/packages/cli/src/commands/generate-multiple-json-column.pin.test.ts @@ -207,15 +207,29 @@ describe('#14829 — `multiple: true` is one answer across all three surfaces', expect(tsColumn('multi_text')).toBe("table.jsonb('multi_text')"); }); - it('nullability still comes from `required`, not from the flag', () => { - const out = generateMigrationSql({ - objects: { probe: { name: 'probe', fields: { tags_req: { type: 'lookup', multiple: true, required: true } } } }, - }); - expect(out).toContain('"tags_req" JSONB NOT NULL'); - const ts = generateMigrationTs({ - objects: { probe: { name: 'probe', fields: { tags_req: { type: 'lookup', multiple: true, required: true } } } }, - }); - expect(ts).toContain("table.jsonb('tags_req').notNullable();"); + /** + * ⚠️ [#16318] The VEHICLE changed, the subject did not. This pin is about + * `multiple` not deciding nullability; `required` was merely how a NOT NULL + * was spelled when it was written. Both generators now take the physical NOT + * NULL from `storage.notNull` and never from `required` (ADR-0113, which took + * `SqlDriver.createColumn` off `required` because binding the DDL to it made + * every post-deploy tightening a destructive migration) — so the constrained + * case is spelled the new way, and the `required`-only case is asserted + * BESIDE it: it must now be nullable in both formats, which is the half that + * would have caught this change silently reverting. + */ + it('nullability comes from `storage.notNull`, not from the flag and not from `required`', () => { + const constrained = { type: 'lookup', multiple: true, storage: { notNull: true } }; + const writeOnly = { type: 'lookup', multiple: true, required: true }; + const config = { objects: { probe: { name: 'probe', fields: { tags_nn: constrained, tags_req: writeOnly } } } }; + + const out = generateMigrationSql(config); + expect(out).toContain('"tags_nn" JSONB NOT NULL'); + expect(out).toMatch(/"tags_req" JSONB(?! NOT NULL)/); + + const ts = generateMigrationTs(config); + expect(ts).toContain("table.jsonb('tags_nn').notNullable();"); + expect(ts).toContain("table.jsonb('tags_req').nullable();"); }); // ── The authority, read where it lives ────────────────────────────────── diff --git a/packages/cli/src/commands/generate-numeric-column-representation.pin.test.ts b/packages/cli/src/commands/generate-numeric-column-representation.pin.test.ts new file mode 100644 index 0000000000..f52c6e4a77 --- /dev/null +++ b/packages/cli/src/commands/generate-numeric-column-representation.pin.test.ts @@ -0,0 +1,230 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * THE #16318 PIN: the NUMERIC column both migration generators emit is the + * numeric column `driver-sql` actually creates, and both read it from the one + * table `packages/spec` states. + * + * ## The defect + * + * One object, seven plain numeric declarations, three producers, driven into + * live PostgreSQL 16.13 and read back out of `information_schema.columns` with + * `numeric_precision` / `numeric_scale` — the half a bare `data_type` read + * hides: + * + * ``` + * driver sql gen ts gen + * number real numeric(18,2) numeric(8,2) + * currency real numeric(18,2) numeric(8,2) + * percent real numeric(5,2) numeric(8,2) + * slider real numeric(18,2) numeric(8,2) + * summary real numeric(18,2) numeric(8,2) + * progress real numeric(5,2) numeric(8,2) + * rating real integer integer + * ``` + * + * 7 of 7 diverged, and six of them THREE ways rather than the two the report + * named — `table.decimal(name)` with no arguments is knex's `decimal(8, 2)`, so + * the two halves of one command never agreed with each other either. A control + * family already unified (#16091: `text` / `email` / `boolean` / `date`) came + * back 0-of-4 divergent in the same run, so AGREE is a reading the instrument + * can produce. After the repair the same command reports 0 of 7. + * + * ## Why this pin reads the spec instead of asserting the numbers + * + * The same reason `generate-string-family-width.pin.test.ts` gives for the + * character widths: the whole shape of this card is "the producers disagree", + * so a pin that transcribed `DECIMAL(65,30)` would re-create the defect one + * layer up and stay green on the day the table moves. Every expectation here is + * derived from `numericColumnFor`, and the membership of the family from + * `NUMERIC_VALUE_TYPES` — neither is listed in this file. + * + * ## The oracle + * + * `SqlDriver.initObjects` on an in-memory better-sqlite3 database, read back + * with `PRAGMA table_info`, exactly as the #16091 pin does it: it runs the real + * `createColumn` chain and reports the column that actually exists. ⚠️ SQLite + * is where the oracle can run in a unit test, and SQLite applies no precision + * and no scale — `ColumnCompiler_SQLite3.prototype.decimal` is the literal + * `'float'`. So the oracle answers the question SQLite can answer (which ARM + * each type takes: the exact-decimal one or the integer one) and the PostgreSQL + * precision/scale claim is carried by the spec-side equality below plus the + * live run recorded in the PR. ⛔ Do not read the oracle as a precision check. + * + * ## The nullability half (ADR-0113, #16294 cause 1) + * + * `SqlDriver.createColumn` emits the physical NOT NULL from `storage.notNull` + * and deliberately not from `required` — its own arm records why: binding the + * DDL to `required` made every post-deploy tightening a destructive migration. + * Both generators were still on `required`. Driven on live PostgreSQL 16.13 + * after the repair, four declaration shapes through all three producers: + * `required` alone is NULLABLE on all three, `storage.notNull` is NOT NULL on + * all three, 0 of 4 diverge, and the probe still distinguishes the two verdicts. + */ + +import { afterAll, describe, expect, it } from 'vitest'; + +import { SqlDriver } from '@objectstack/driver-sql'; +import { + NUMERIC_VALUE_TYPES, + numericColumnFor, + type NumericColumnRepresentation, +} from '@objectstack/spec/data'; + +import { generateMigrationSql, generateMigrationTs } from './generate.js'; + +const NUMERIC_TYPES = [...NUMERIC_VALUE_TYPES].sort(); + +/** + * ⭐ THE REAL CHAIN, widened exactly as `generate-string-family-width.pin.test` + * widens it: `protected` is a compile-time visibility rule, so a subclass can + * publish the driver's own `knex` without copying a character of its logic. + * `initObjects` dispatches every field through `createColumn` and hands the + * result to knex; `PRAGMA table_info` reports the column that then exists. + */ +class DriverOracle extends SqlDriver { + public async createdColumns(object: { name: string; fields?: Record }): Promise> { + await this.initObjects([object as never]); + const rows = (await this.knex.raw(`PRAGMA table_info("${object.name}")`)) as Array<{ + name: string; + type: string; + }>; + return new Map(rows.map((row) => [row.name, row.type])); + } +} + +const ORACLE = new DriverOracle({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, +}); + +afterAll(async () => { + await ORACLE.disconnect(); +}); + +/** The answer this file is allowed to expect — resolved, never transcribed. */ +function stated(type: string): NumericColumnRepresentation { + const answer = numericColumnFor(type); + if (!answer) throw new Error(`packages/spec states no column for the numeric type '${type}'`); + return answer; +} + +function objectOf(types: readonly string[]): Record { + const fields: Record = {}; + for (const t of types) fields[`f_${t}`] = { type: t }; + return { name: 'num_zoo', fields }; +} + +describe('#16318 — both migration formats emit the stated numeric column', () => { + it('the family is non-empty and this file did not invent its membership', () => { + // Non-vacuity: every assertion below loops over NUMERIC_TYPES, so an empty + // set would pass every one of them while measuring nothing. + expect(NUMERIC_TYPES.length).toBeGreaterThanOrEqual(7); + expect(NUMERIC_TYPES).toContain('currency'); + expect(NUMERIC_TYPES).toContain('rating'); + }); + + it('the sql format emits the stated column for every member', () => { + const sql = generateMigrationSql({ objects: { num_zoo: objectOf(NUMERIC_TYPES) } }); + for (const type of NUMERIC_TYPES) { + const answer = stated(type); + const expected = + answer.kind === 'integer' ? 'INTEGER' : `DECIMAL(${answer.precision},${answer.scale})`; + expect(sql, type).toContain(`"f_${type}" ${expected}`); + } + // The two shapes this card removed must be gone from the whole emission, + // not merely absent from the rows above. + expect(sql).not.toContain('DECIMAL(18,2)'); + expect(sql).not.toContain('DECIMAL(5,2)'); + }); + + it('the typescript format emits the stated column for every member', () => { + const ts = generateMigrationTs({ objects: { num_zoo: objectOf(NUMERIC_TYPES) } }); + for (const type of NUMERIC_TYPES) { + const answer = stated(type); + const expected = + answer.kind === 'integer' + ? `table.integer('f_${type}')` + : `table.decimal('f_${type}', ${answer.precision}, ${answer.scale})`; + expect(ts, type).toContain(expected); + } + // Knex's argument-less `decimal` is `decimal(8, 2)` — the third answer in + // the divergence, and the one no reader of this file would guess. + expect(ts).not.toMatch(/table\.decimal\('f_[a-z]+'\)/); + }); + + it('the two formats agree with each other on every member', () => { + const sql = generateMigrationSql({ objects: { num_zoo: objectOf(NUMERIC_TYPES) } }); + const ts = generateMigrationTs({ objects: { num_zoo: objectOf(NUMERIC_TYPES) } }); + for (const type of NUMERIC_TYPES) { + const answer = stated(type); + if (answer.kind === 'integer') { + expect(sql, type).toContain(`"f_${type}" INTEGER`); + expect(ts, type).toContain(`table.integer('f_${type}')`); + } else { + expect(sql, type).toContain(`"f_${type}" DECIMAL(${answer.precision},${answer.scale})`); + expect(ts, type).toContain(`table.decimal('f_${type}', ${answer.precision}, ${answer.scale})`); + } + } + }); + + /** + * The ORACLE. Which ARM the driver puts each type in — the only half of the + * claim SQLite can carry, see the docblock. + */ + it('the driver puts every member in the arm the spec states', async () => { + const byName = await ORACLE.createdColumns(objectOf(NUMERIC_TYPES) as never); + + // Non-vacuity: the read must have found the columns at all. + for (const type of NUMERIC_TYPES) expect(byName.has(`f_${type}`), `f_${type} missing`).toBe(true); + + for (const type of NUMERIC_TYPES) { + const answer = stated(type); + // knex compiles BOTH `decimal(p, s)` and `float` to the literal `float` + // on SQLite, which is exactly the measurement that makes this move + // affinity-neutral for the six exact-decimal members. + expect(byName.get(`f_${type}`)?.toLowerCase(), type).toBe( + answer.kind === 'integer' ? 'integer' : 'float', + ); + } + // The discriminating control: the two arms must not have collapsed into + // one, or "the driver agrees" would be a constant rather than a reading. + expect(new Set(NUMERIC_TYPES.map((t) => byName.get(`f_${t}`)?.toLowerCase())).size).toBe(2); + }); +}); + +describe('#16318 / ADR-0113 — both formats take NOT NULL from `storage.notNull`', () => { + const FIELDS = { + f_required_only: { type: 'text', required: true }, + f_storage_only: { type: 'text', storage: { notNull: true } }, + f_both: { type: 'text', required: true, storage: { notNull: true } }, + f_neither: { type: 'text' }, + }; + const config = { objects: { nn: { name: 'nn', fields: FIELDS } } }; + + it('the sql format constrains exactly the columns the driver constrains', () => { + const sql = generateMigrationSql(config); + expect(sql).toContain('"f_storage_only" TEXT NOT NULL'); + expect(sql).toContain('"f_both" TEXT NOT NULL'); + // `required` alone is the WRITE-time contract; it must not reach the DDL. + expect(sql).toMatch(/"f_required_only" TEXT(?! NOT NULL)/); + expect(sql).toMatch(/"f_neither" TEXT(?! NOT NULL)/); + }); + + it('the typescript format constrains exactly the same columns', () => { + const ts = generateMigrationTs(config); + expect(ts).toContain(`table.text('f_storage_only').notNullable()`); + expect(ts).toContain(`table.text('f_both').notNullable()`); + expect(ts).toContain(`table.text('f_required_only').nullable()`); + expect(ts).toContain(`table.text('f_neither').nullable()`); + }); + + it('the probe distinguishes its two verdicts', () => { + // Without this the two tests above would pass against a generator that + // emitted NOT NULL for everything, or for nothing. + const ts = generateMigrationTs(config); + expect(ts).toContain('.notNullable()'); + expect(ts).toContain('.nullable()'); + }); +}); diff --git a/packages/cli/src/commands/generate.ts b/packages/cli/src/commands/generate.ts index fe8d04fd50..f8bce03051 100644 --- a/packages/cli/src/commands/generate.ts +++ b/packages/cli/src/commands/generate.ts @@ -17,7 +17,7 @@ import type { FieldType } from '@objectstack/spec/data'; // `computeTenantField` — are spelled here in the driver's own terms ON TOP of // these, so the part that can be shared is shared and only the part that // genuinely lives on `driver-sql` is mirrored. -import { isTenancyDisabled, isUniqueDeclared } from '@objectstack/spec/data'; +import { isTenancyDisabled, isUniqueDeclared, numericColumnFor } from '@objectstack/spec/data'; import { printHeader, printSuccess, printError, printInfo, printStep, createTimer, CLI_ALIAS } from '../utils/format.js'; import { metadataFileName } from '../utils/metadata-file-name.js'; import { findEmissionParseFailures } from '../utils/emitted-source-parses.js'; @@ -1122,9 +1122,18 @@ const FIELD_TYPE_SQL_MAP: Record = { richtext: 'TEXT', html: 'TEXT', markdown: 'TEXT', - number: 'DECIMAL(18,2)', - currency: 'DECIMAL(18,2)', - percent: 'DECIMAL(5,2)', + // #16318 — the NUMERIC family's seven members are RESOLVED, never written + // here. `DECIMAL(18,2)` / `DECIMAL(5,2)` were this file's own numbers and no + // other producer ever agreed with them: measured on live PostgreSQL 16.13, + // one object through all three producers, `number` was `real` on the driver, + // `numeric(18,2)` from this map and `numeric(8,2)` from the typescript format + // below — a THREE-way split, every arm of it lossy in a different direction. + // These entries exist so this map stays total over `FieldType`; the ANSWER is + // {@link numericSqlType} over `packages/spec`'s own table, which + // `SqlDriver.createColumn` reads too. + number: numericSqlType('number'), + currency: numericSqlType('currency'), + percent: numericSqlType('percent'), boolean: 'BOOLEAN', date: 'DATE', // #15521 — TIMESTAMPTZ, not TIMESTAMP, for the same reason and with the same @@ -1200,7 +1209,7 @@ const FIELD_TYPE_SQL_MAP: Record = { // a design-token name) is a value the platform stores and a table generated // for the same object refuses. color: 'VARCHAR(255)', - rating: 'INTEGER', + rating: numericSqlType('rating'), // #14828 — `vector` is in STRUCTURED_JSON_TYPES, hence in the driver's // `JSON_COLUMN_TYPES`. `VECTOR` was also not portable: it needs pgvector and // does not exist on MySQL or SQLite. @@ -1224,11 +1233,16 @@ const FIELD_TYPE_SQL_MAP: Record = { // `driver-sql`'s `JSON_COLUMN_TYPES`, which is seeded from this same class. checkboxes: 'JSONB', tags: 'JSONB', - // NUMERIC_VALUE_TYPES. `progress` takes `percent`'s narrower shape because it - // is the same 0-100 quantity; `slider` and `summary` are open-range. - slider: 'DECIMAL(18,2)', - progress: 'DECIMAL(5,2)', - summary: 'DECIMAL(18,2)', + // NUMERIC_VALUE_TYPES — #16318, resolved like the four above. `progress` + // used to take `percent`'s NARROWER shape here because it is the same 0-100 + // quantity; it still shares `percent`'s answer, and the shared answer is now + // the wide one. Measured, and the reason the narrow one could not stay: a + // `percent` stores a 0-1 FRACTION unless the field declares `max > 1` + // (`percentScaleOf`), so the legitimate 33.333% the ruling names reaches the + // column as `0.33333`, and `numeric(5,2)` ROUNDED it to `0.33`. + slider: numericSqlType('slider'), + progress: numericSqlType('progress'), + summary: numericSqlType('summary'), // REFERENCE_VALUE_TYPES: the stored value is the related record's id, so the // width belongs to the TARGET's id column, never to this field. #14828 read // that derivation off the driver and applied it: the target's `id` column is @@ -1273,6 +1287,50 @@ const FIELD_TYPE_SQL_MAP: Record = { */ const STRING_FAMILY_TYPES: ReadonlySet = new Set(['email', 'url', 'phone', 'password']); +/** + * The NUMERIC family's column, in this format's SQL vocabulary (#16318). + * + * ⛔ Never a transcription. The precision, the scale and the per-type answer + * all live in `packages/spec`'s {@link numericColumnFor}, which + * `SqlDriver.createColumn` reads too — that shared table IS the repair, and a + * literal `DECIMAL(18,2)` here would re-create the divergence one layer up. + * This function only spells the answer; it decides nothing. + * + * It throws rather than falling back, and the throw is the point: an undefined + * answer for one of the seven literals its callers pass would mean this file's + * vocabulary and `NUMERIC_VALUE_TYPES` have parted. A fallback string would + * emit a column that silently disagrees with the platform's — the exact defect + * #16318 closes — so the failure is made loud instead. `packages/spec`'s + * `numeric-column-representation.test.ts` fails first, in CI, in both + * directions. + */ +function numericSqlType(type: string): string { + const numeric = numericColumnFor(type); + if (numeric === undefined) { + throw new Error( + `generate: '${type}' is not in NUMERIC_VALUE_TYPES, so packages/spec states no column for it. ` + + 'Add it to the numeric physical-representation table, or stop asking this resolver for it.', + ); + } + return numeric.kind === 'integer' ? 'INTEGER' : `DECIMAL(${numeric.precision},${numeric.scale})`; +} + +/** + * ADR-0113's physical NOT NULL, spelled the way `SqlDriver.createColumn` + * spells it: `(field as { storage?: { notNull?: boolean } }).storage?.notNull`. + * + * ⛔ NOT `required`. The driver was deliberately taken off that key, and its + * own arm records why: "`required` is the write-time contract enforced by the + * record validator at the engine seam, and binding the DDL to it made every + * post-deploy tightening a destructive migration". Both generators stayed on + * `required`, so a scaffolded table constrained columns the platform's own + * table leaves nullable — #16294 cause 1, which this unblocks. That card's + * other two causes are not addressed here. + */ +function declaredNotNull(field: unknown): boolean { + return (field as { storage?: { notNull?: boolean } } | undefined)?.storage?.notNull === true; +} + /** * The widest `varchar(n)` any dialect this platform speaks will declare — * `SqlDriver.MAX_VARCHAR_CHARS`, whose own comment records the measurement @@ -1707,7 +1765,10 @@ export function generateMigrationSql(config: Record): string { // returns without emitting one and `schema-drift.ts`'s `fieldHasColumn` // answers false for it, so a column here is one the runtime never writes. if (sqlType === null) continue; - const notNull = fieldDef.required ? ' NOT NULL' : ''; + // [#16318 / ADR-0113] The physical NOT NULL comes from the EXPLICIT + // storage constraint, never from `required`. See {@link declaredNotNull} + // for the driver's own recorded reason; ⛔ do not restate it here. + const notNull = declaredNotNull(fieldDef) ? ' NOT NULL' : ''; fieldLines.push(` "${fieldName}" ${sqlType}${notNull}`); } @@ -1821,7 +1882,10 @@ export function generateMigrationTs(config: Record): string { for (const [fieldName, fieldDef] of Object.entries(fields)) { const fType = String(fieldDef.type || 'text'); - const required = fieldDef.required ? '.notNullable()' : '.nullable()'; + // [#16318 / ADR-0113] `storage.notNull`, never `required` — the same + // move, for the same recorded reason, as the sql format above. The local + // name is kept so the emitter below reads unchanged. + const required = declaredNotNull(fieldDef) ? '.notNullable()' : '.nullable()'; // #14829 - `multiple` before the type, exactly as `SqlDriver.createColumn` // does it: the driver short-circuits on the flag above its own per-type @@ -1905,15 +1969,29 @@ export function generateMigrationTs(config: Record): string { : `table.string('${fieldName}', ${keyable})`; break; } - case 'number': case 'currency': case 'percent': - // #14657 — NUMERIC_VALUE_TYPES: `valueSchemaFor` gives all of these - // `z.number()`, and `driver-sql` gives them a float column. - case 'slider': case 'progress': case 'summary': - colMethod = `table.decimal('${fieldName}')`; - break; - case 'rating': - colMethod = `table.integer('${fieldName}')`; + // #16318 — NUMERIC_VALUE_TYPES, resolved from `packages/spec`'s own + // physical-representation table, which `SqlDriver.createColumn` and the + // sql format above read too. + // + // ⚠️ `table.decimal(name)` with NO arguments — what this arm used to + // emit — is knex's `decimal(8, 2)`, not an unconstrained `numeric`. + // Measured on live PostgreSQL 16.13, that column REFUSED `1234567.89` + // outright: a money value the platform stores today could not be stored + // in a table this format generated for the same object. It never + // matched the sql format's own `DECIMAL(18,2)` either, so the two halves + // of one command disagreed with each other as well as with the driver. + case 'number': case 'currency': case 'percent': case 'rating': + case 'slider': case 'progress': case 'summary': { + const numeric = numericColumnFor(fType); + // ⛔ Not a fallback spelling — see {@link numericSqlType} for why an + // undefined answer here is made loud rather than papered over. + if (numeric === undefined) throw new Error(`generate: no column stated for numeric type '${fType}'`); + colMethod = + numeric.kind === 'integer' + ? `table.integer('${fieldName}')` + : `table.decimal('${fieldName}', ${numeric.precision}, ${numeric.scale})`; break; + } case 'boolean': // #14657 — BOOLEAN_VALUE_TYPES; `driver-sql` shares one arm for the pair. case 'toggle': diff --git a/packages/drivers/driver-sql/src/sql-driver-16318-numeric-representation-live-dialects.test.ts b/packages/drivers/driver-sql/src/sql-driver-16318-numeric-representation-live-dialects.test.ts new file mode 100644 index 0000000000..1ae1d83553 --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-16318-numeric-representation-live-dialects.test.ts @@ -0,0 +1,281 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#16318] The NUMERIC family's stated physical representation, read off the + * SERVER's own catalog on the two dialects the representation exists for. + * + * ## Why this file exists as well as the SQLite one + * + * `sql-driver-16318-numeric-representation.test.ts` is honest about its own + * reach and says so in its head note: SQLite applies neither precision nor + * scale, so it can pin the ARM (`float` vs `integer`) and the storage class and + * nothing else. Every claim the change is actually about — a + * `numeric(65,30)` column, a `rating` that refuses or rounds a half star, and a + * read path that hands back a JS `number` where node-postgres and mysql2 hand + * back a STRING — is a PostgreSQL/MySQL claim, and on SQLite it cannot fire at + * all: SQLite never returns a string for a `float` column, so the read-path + * assertion over there passes on a driver that lost the coercion entirely. + * + * ⇒ Without this file the every-dialect `formatOutput` move has zero automated + * coverage on the two dialects it was made for, and the precision/scale the + * whole table decides is prose in a pull request. + * + * ## What is asserted, and against what authority + * + * `information_schema.columns` — the server's own catalog, spelled the same way + * on both dialects — read for `numeric_precision` / `numeric_scale`, compared + * against `packages/spec`'s {@link numericColumnFor} rather than against a + * transcribed literal. A second width table here would re-create the very drift + * #16318 closes; what is pinned is that the SERVER agrees with the SPEC. + * + * ⚠️ `rating`'s fractional disposition is asserted PER DIALECT because the two + * dialects genuinely differ, and stating one answer for both is the defect this + * cell was added for: PostgreSQL REFUSES `4.5` into an `integer` column, and + * MySQL does not refuse — it ROUNDS to `5`. Both are silent-alteration-class + * facts a changelog must not average into one sentence. + * + * Opt-in — these need real servers: + * + * OS_TEST_MYSQL_URL=mysql://root:root@127.0.0.1:3306/conformance \ + * OS_TEST_POSTGRES_URL=postgres://postgres:postgres@127.0.0.1:5432/postgres \ + * pnpm --filter @objectstack/driver-sql test + * + * Unprovisioned, each cell reports itself as a named SKIP and is a FAILURE + * under `OS_EXPECT_LIVE_DIALECT_MATRIX=1` — the "Temporal Conformance (live PG + * + MySQL)" job, which runs this package's whole test script. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { NUMERIC_VALUE_TYPES, numericColumnFor } from '@objectstack/spec/data'; +import { SqlDriver } from './sql-driver.js'; +import { + MYSQL_CELL, + PG_CELL, + currentLiveSchema, + declareDialectCell, + type DialectCell, +} from './live-dialect-matrix.testkit.js'; + +const T = 'os16318_numeric'; + +/** Every member of the family, from the spec's own membership authority. */ +const NUMERIC_TYPES = [...NUMERIC_VALUE_TYPES].sort(); + +/** One object carrying the whole family, one column per member. */ +const numericObject = () => { + const fields: Record = { name: { type: 'text', maxLength: 64 } }; + for (const t of NUMERIC_TYPES) fields[`f_${t}`] = { type: t }; + return { name: T, fields }; +}; + +/** What the SERVER says about a column, normalised across the two dialects. */ +interface CatalogColumn { + dataType: string; + precision: number | null; + scale: number | null; +} + +/** + * Read the catalog through the driver's own connection. + * + * `information_schema.columns` is standard on both dialects, and `table_schema` + * is the per-file isolation name in both — a SCHEMA on PostgreSQL, a DATABASE + * on MySQL, which is the same concept there. The select list is ALIASED so the + * two client libraries hand back the same keys (MySQL's catalog spells its + * columns upper-case). + */ +async function catalogColumns(driver: SqlDriver, schema: string): Promise> { + // The schema name is derived by `liveSchemaNameFor`, which refuses anything + // outside /^[a-z][a-z0-9_]*$/ — so it cannot carry a quote into this SQL. + const res: any = await driver.execute( + `select column_name as c, data_type as d, numeric_precision as p, numeric_scale as s ` + + `from information_schema.columns ` + + `where table_schema = '${schema}' and table_name = '${T}'`, + ); + const rows: any[] = Array.isArray(res) && Array.isArray(res[0]) ? res[0] : (res?.rows ?? res); + const out = new Map(); + for (const r of rows) { + const name = String(r.c ?? r.C ?? r.column_name ?? r.COLUMN_NAME); + out.set(name, { + dataType: String(r.d ?? r.D).toLowerCase(), + precision: r.p === null || r.p === undefined ? null : Number(r.p), + scale: r.s === null || r.s === undefined ? null : Number(r.s), + }); + } + return out; +} + +for (const cell of [PG_CELL, MYSQL_CELL]) { + declareDialectCell(cell, 'numeric column representation (#16318)', (c: DialectCell) => { + describe(`numeric column representation on ${c.label} (#16318)`, () => { + let live: SqlDriver; + + afterEach(async () => { + await live?.execute(`drop table if exists ${T}`).catch(() => {}); + await live?.disconnect().catch(() => {}); + }); + + const boot = async () => { + live = new SqlDriver(c.config()); + await live.execute(`drop table if exists ${T}`).catch(() => {}); + await live.initObjects([numericObject()] as never); + }; + + it('creates the exact column `packages/spec` states — precision and scale read off the server', async () => { + await boot(); + const cols = await catalogColumns(live, currentLiveSchema()); + + // Non-vacuity, first: the catalog read really answered. Without this an + // empty result set would satisfy every loop below. + expect(NUMERIC_TYPES.length, 'the family is empty').toBeGreaterThanOrEqual(7); + for (const t of NUMERIC_TYPES) expect(cols.has(`f_${t}`), `f_${t} absent from the catalog`).toBe(true); + + for (const t of NUMERIC_TYPES) { + const want = numericColumnFor(t); + expect(want, t).toBeDefined(); + const got = cols.get(`f_${t}`)!; + if (want!.kind === 'integer') { + // PostgreSQL says `integer`, MySQL says `int` — one substring, no + // second table. + expect(got.dataType, t).toMatch(/^int(eger)?$/); + expect(got.scale, `${t} scale`).toBe(0); + } else { + expect(got.dataType, t).toMatch(/^(numeric|decimal)$/); + // ⭐ THE ASSERTION the whole card is about, against the SPEC's + // numbers rather than a transcribed pair. + expect(got.precision, `${t} precision`).toBe(want!.precision); + expect(got.scale, `${t} scale`).toBe(want!.scale); + } + } + + // The two arms really are two — a chain that answered `integer` (or + // `numeric`) for everything satisfies each assertion above in isolation. + expect(new Set(NUMERIC_TYPES.map((t) => cols.get(`f_${t}`)!.dataType)).size).toBe(2); + }); + + it('reads every member back as a JS number, where the client library hands back a string', async () => { + await boot(); + + // ⛔ The discriminating value: node-postgres parses `numeric` to a + // STRING and mysql2 does the same for `DECIMAL`, so without + // `formatOutput`'s every-dialect `numericFields` coercion these come + // back as `'0.333330000000000000000000000000'` and the wire contract + // (`z.number().finite()`) is broken. On SQLite this assertion cannot + // fire at all, which is why it lives here. + const written: Record = {}; + for (const t of NUMERIC_TYPES) written[`f_${t}`] = t === 'rating' ? 4 : 0.33333; + await live.create(T, { id: 'n1', name: 'rt', ...written }); + + const [back]: any[] = await live.find(T, { filters: ['name', '=', 'rt'] } as never); + expect(back, 'the row did not come back').toBeDefined(); + for (const [k, v] of Object.entries(written)) { + expect(typeof back[k], `${k} came back as ${typeof back[k]}`).toBe('number'); + expect(back[k], k).toBe(v); + } + }); + + it('holds a value the previous `real` column lost, and the previous narrow decimals rounded', async () => { + await boot(); + + // `1234567.89` is the money value binary32 could not hold: measured on + // a `real` column it read back `1234567.9`. `33.333` is the percent the + // sql format's `numeric(5,2)` rounded to `33.33`. + await live.create(T, { id: 'n2', name: 'exact', f_currency: 1234567.89, f_percent: 33.333 }); + const [row]: any[] = await live.find(T, { filters: ['name', '=', 'exact'] } as never); + expect(row.f_currency).toBe(1234567.89); + expect(row.f_percent).toBe(33.333); + }); + + it('disposes of a fractional star count the way THIS dialect does — refuse on PostgreSQL, round on MySQL', async () => { + await boot(); + + // ⚠️ The two dialects genuinely differ, and a changelog that states one + // answer for both is the finding this cell was added for. Asserted per + // dialect, never averaged. + const outcome = await live + .create(T, { id: 'n3', name: 'half', f_rating: 4.5 }) + .then(() => 'accepted' as const) + .catch(() => 'refused' as const); + + if (c.id === 'pg') { + expect(outcome, 'PostgreSQL accepted a fractional value into an integer column').toBe('refused'); + const rows: any[] = await live.find(T, { filters: ['name', '=', 'half'] } as never); + expect(rows, 'the refused row was written anyway').toHaveLength(0); + } else { + expect(outcome, 'MySQL refused a fractional value it is documented to round').toBe('accepted'); + const [row]: any[] = await live.find(T, { filters: ['name', '=', 'half'] } as never); + // ⭐ The silent alteration itself: no error, and the star count the + // caller wrote is NOT the star count the database now holds. + expect(row.f_rating, 'MySQL did not round 4.5 to 5').toBe(5); + expect(row.f_rating).not.toBe(4.5); + } + }); + }); + }); +} + +// ── The "new tables only" bound on the READ path (#16318 F4) ──────────────── +// +// Moving `formatOutput`'s numeric coercion off the SQLite-only arm is what the +// exact-decimal column forced, and `numericFields` carries the driver-internal +// aliases `integer` / `int` / `float` — which is how an EXTERNAL, introspected +// table's columns reach this driver. PostgreSQL is where that matters: node- +// postgres hands back `int8` as a STRING precisely because it does not fit a JS +// double, so an unscoped pass would `Number()` it and silently round above +// 2^53 on a table this change never created. +// +// ⛔ Not a MySQL cell: mysql2 hands `BIGINT` back as a JS number already, so +// there is no string for any pass to touch and the reading would be vacuous. + +const EXT_TABLE = 'os16318_ext_bigint'; +const EXT_OBJECT = 'os16318_ext'; +/** 2^53 + 1 — the smallest integer a JS double cannot represent. */ +const BEYOND_DOUBLE = '9007199254740993'; + +declareDialectCell(PG_CELL, 'numeric read-path scope (#16318)', (c: DialectCell) => { + describe(`the numeric read coercion leaves an EXISTING external bigint alone on ${c.label} (#16318)`, () => { + let live: SqlDriver; + + afterEach(async () => { + await live?.execute(`drop table if exists ${EXT_TABLE}`).catch(() => {}); + await live?.disconnect().catch(() => {}); + }); + + it('hands back the bigint string unrounded, while an authorable numeric field IS coerced', async () => { + live = new SqlDriver(c.config()); + await live.execute(`drop table if exists ${EXT_TABLE}`).catch(() => {}); + // A table this change did not create, shaped the way an introspected one + // is: a `bigint` under a driver ALIAS field type, beside an authorable + // `number` under the exact-decimal column this change does create. + await live.execute( + `create table ${EXT_TABLE} (id varchar(64) primary key, big bigint, amount numeric(65,30))`, + ); + await live.execute( + `insert into ${EXT_TABLE} (id, big, amount) values ('e1', ${BEYOND_DOUBLE}, 12.5)`, + ); + + live.registerExternalObject!({ + name: EXT_OBJECT, + external: { remoteName: EXT_TABLE }, + fields: { big: { type: 'integer' }, amount: { type: 'number' } }, + } as never); + + const [row]: any[] = await live.find(EXT_OBJECT, { filters: ['id', '=', 'e1'] } as never); + expect(row, 'the external row did not come back').toBeDefined(); + + // ⭐ THE ASSERTION. `Number('9007199254740993')` is 9007199254740992 — a + // silent one-off on an existing column, outside the "new tables only" + // bound the ruling drew. The value must arrive as the server sent it. + expect(typeof row.big, 'a bigint under a driver alias was coerced through a JS double').toBe( + 'string', + ); + expect(row.big).toBe(BEYOND_DOUBLE); + + // ⛔ Non-vacuity, and the discriminating half: the SAME pass, in the SAME + // read, still coerces the authorable numeric field — otherwise this test + // would pass on a driver that lost the coercion altogether. + expect(typeof row.amount, 'the authorable numeric field was NOT coerced').toBe('number'); + expect(row.amount).toBe(12.5); + }); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver-16318-numeric-representation.test.ts b/packages/drivers/driver-sql/src/sql-driver-16318-numeric-representation.test.ts new file mode 100644 index 0000000000..68c5b875e4 --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-16318-numeric-representation.test.ts @@ -0,0 +1,132 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#16318] The NUMERIC family's column comes from `packages/spec`'s stated + * physical representation, and the SQLite consequence of that move is pinned + * per type — the constraint the card itself raised: + * + * > SQLite affinity is what put `rating`/`slider`/`progress` in the driver's + * > float arm in the first place, so any move has to be judged there too. + * + * `createColumn`'s float arm records the leak it was written to defeat: without + * an explicit case these types fell to `table.string`, the column took TEXT + * affinity, and SQLite stored `'4'` rather than `4`. This file pins that the + * leak stays defeated, in the only terms that decide it — the storage class + * SQLite actually records. + * + * ## What the move does on SQLite, per type — measured, not argued + * + * knex compiles `table.decimal(name, p, s)` and `table.float(name)` to the + * IDENTICAL `float` column on SQLite (`ColumnCompiler_SQLite3.prototype.decimal` + * is the literal `'float'`), so: + * + * - the six exact-decimal members emit BYTE-IDENTICAL SQLite DDL to the float + * arm they leave and keep REAL affinity; + * - `rating` moves to INTEGER affinity, where `4` is stored as the integer + * `4` rather than the real `4.0`, and `4.5` is still accepted as a REAL — + * SQLite refuses no fractional value, so nothing this dialect accepts today + * stops being accepted. + * + * The exactness the move buys is a PostgreSQL/MySQL property; SQLite applies no + * precision and no scale and behaves exactly as it does today. + * + * ## The read half, which is what makes the move safe on the server dialects + * + * node-postgres parses `real` to a JS number and `numeric` to a STRING, and + * mysql2 does the same for `DECIMAL`. `formatOutput`'s `numericFields` pass was + * SQLite-only on the premise that string-valued numerics come only from legacy + * TEXT-affinity columns; #16318 falsified that premise and moved the pass to + * every dialect. Pinned here through the driver's own read door. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { NUMERIC_VALUE_TYPES, numericColumnFor } from '@objectstack/spec/data'; +import { SqlDriver } from './index.js'; + +const NUMERIC_TYPES = [...NUMERIC_VALUE_TYPES].sort(); + +class Probe extends SqlDriver { + public async declaredColumns(table: string): Promise> { + const rows = (await this.knex.raw(`PRAGMA table_info("${table}")`)) as Array<{ + name: string; + type: string; + }>; + return new Map(rows.map((r) => [r.name, r.type.toLowerCase()])); + } + + public async storageClass(table: string, column: string): Promise> { + return (await this.knex.raw( + `select typeof("${column}") as t, "${column}" as v from "${table}" where "${column}" is not null`, + )) as Array<{ t: string; v: unknown }>; + } +} + +describe('#16318 — the numeric family on SQLite', () => { + let driver: Probe; + + beforeEach(async () => { + driver = new Probe({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + const fields: Record = { name: { type: 'text' } }; + for (const t of NUMERIC_TYPES) fields[`f_${t}`] = { type: t }; + await driver.initObjects([{ name: 'zoo', fields } as never]); + }); + + afterEach(async () => { + await driver.disconnect(); + }); + + it('declares the arm the spec states, and the two arms have not collapsed into one', async () => { + const cols = await driver.declaredColumns('zoo'); + // Non-vacuity: without this an empty PRAGMA read would pass every loop. + expect(NUMERIC_TYPES.length).toBeGreaterThanOrEqual(7); + for (const t of NUMERIC_TYPES) expect(cols.has(`f_${t}`), `f_${t}`).toBe(true); + + for (const t of NUMERIC_TYPES) { + const answer = numericColumnFor(t); + expect(answer, t).toBeDefined(); + // The whole SQLite claim: the exact-decimal members declare `float` — + // the same string the float arm they left declared. + expect(cols.get(`f_${t}`), t).toBe(answer!.kind === 'integer' ? 'integer' : 'float'); + } + expect(new Set(NUMERIC_TYPES.map((t) => cols.get(`f_${t}`))).size).toBe(2); + // ⛔ The fossil's own leak: nothing in this family may declare a character + // column, which is what put three of these types in the float arm. + for (const t of NUMERIC_TYPES) expect(cols.get(`f_${t}`), t).not.toMatch(/char|text|clob/); + }); + + it('keeps every member out of TEXT storage, and rating takes INTEGER storage', async () => { + const row: Record = { name: 'r' }; + for (const t of NUMERIC_TYPES) row[`f_${t}`] = 4; + await driver.create('zoo', row); + + for (const t of NUMERIC_TYPES) { + const [cell] = await driver.storageClass('zoo', `f_${t}`); + // The fossil's leak, pinned as the storage class rather than the type name. + expect(cell.t, t).not.toBe('text'); + const answer = numericColumnFor(t)!; + expect(cell.t, t).toBe(answer.kind === 'integer' ? 'integer' : 'real'); + } + }); + + it('rating still accepts a fractional value on SQLite — the refusal it gains is server-side', async () => { + await driver.create('zoo', { name: 'half', f_rating: 4.5 }); + const [cell] = await driver.storageClass('zoo', 'f_rating'); + expect(cell.t).toBe('real'); + expect(cell.v).toBe(4.5); + }); + + it('reads every member back as a JS number', async () => { + const written: Record = {}; + for (const t of NUMERIC_TYPES) written[`f_${t}`] = t === 'rating' ? 4 : 33.333; + await driver.create('zoo', { name: 'rt', ...written }); + const [back] = await driver.find('zoo', { filters: ['name', '=', 'rt'] } as never); + for (const [k, v] of Object.entries(written)) { + expect(typeof back[k], k).toBe('number'); + expect(back[k], k).toBe(v); + } + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 0e9b25dd2e..40d5707258 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -22,6 +22,10 @@ import { parseAutonumberFormat, renderAutonumber, resolveAutonumberFormat, readA // `AggregationNodeSchema.function` actually admits. import { AggregationFunction, emptyGroupValueFor } from '@objectstack/spec/data'; import { STRUCTURED_JSON_TYPES, FILE_REFERENCE_TYPES, MULTI_OPTION_TYPES, NUMERIC_VALUE_TYPES } from '@objectstack/spec/data'; +// [#16318] The per-field-type physical representation of the NUMERIC family. +// `os generate migration` reads the SAME table, in both of its formats — that +// shared table IS the repair, so ⛔ never restate one of its numbers here. +import { numericColumnFor } from '@objectstack/spec/data'; // [#5659] The Filter Protocol's boolean identity reduction — `$and: []` is TRUE, // `$or: []` is FALSE, `{}` is a TRUE disjunct, `$not: {}` is FALSE. One // implementation for all four consumers, proven against the same @@ -245,6 +249,13 @@ const JSON_COLUMN_TYPES = new Set([ * column-type switch (these map to INTEGER/REAL columns) and the read-side * coercion registry (`numericFields`). * + * ⚠️ [#16318] The read coercion no longer reads this whole set on every + * dialect. The three ALIASES below are how an external, introspected column + * reaches the driver, and on PostgreSQL a `bigint` arrives as a STRING; the + * server-dialect arm therefore reads {@link SqlDriver.numericValueFields}, the + * authorable `NUMERIC_VALUE_TYPES` half, and SQLite keeps this full set for the + * legacy TEXT-affinity repair described below. The DDL switch is unchanged. + * * The read coercion exists so the fix is robust on SQLite even when the column * predates it: a `rating`/`slider`/`progress` column created before #2025 has * TEXT affinity and returns '4' not 4, and SQLite never alters a column's type @@ -4376,6 +4387,30 @@ export class SqlDriver implements IDataDriver { protected jsonFields: Record = {}; protected booleanFields: Record = {}; protected numericFields: Record = {}; + /** + * [#16318] The subset of {@link numericFields} whose field type is an + * AUTHORABLE numeric one (`NUMERIC_VALUE_TYPES`), with the driver-internal + * SQL aliases `integer` / `int` / `float` deliberately left out. + * + * Why a second registry rather than a narrower first one: `numericFields` is + * read by three other seams — the presentation-kind door, the cross-field + * comparability door, and the shard aliasing — and every one of them is about + * "this column holds a number", which the aliases do. Only the READ COERCION + * needed narrowing, and it needed it on exactly one axis. + * + * ⚠️ The axis is EXISTING columns. Moving the coercion off the SQLite-only + * arm (see `formatOutput`) is what the exact-decimal column forced, and the + * aliases are how an EXTERNAL, introspected table's columns reach this + * driver — a PostgreSQL `bigint`, which node-postgres hands back as a STRING + * precisely because it does not fit a JS double. Coercing those through + * `Number()` would silently round above 2^53 on a table this change never + * created, which is outside the "new tables only" bound the ruling drew + * (「不考虑现有数据」). So on the server dialects the coercion applies to the + * seven authorable numeric types and to nothing else; SQLite keeps the wider + * set, because that is where the legacy TEXT-affinity repair this pass was + * originally written for actually lives. + */ + protected numericValueFields: Record = {}; protected dateFields: Record> = {}; protected datetimeFields: Record> = {}; /** @@ -9631,6 +9666,7 @@ export class SqlDriver implements IDataDriver { this.jsonFields[shard] = this.jsonFields[base] ?? []; this.booleanFields[shard] = this.booleanFields[base] ?? []; this.numericFields[shard] = this.numericFields[base] ?? []; + this.numericValueFields[shard] = this.numericValueFields[base] ?? []; this.autoNumberFields[shard] = this.autoNumberFields[base] ?? []; if (this.dateFields[base]) this.dateFields[shard] = this.dateFields[base]; if (this.datetimeFields[base]) this.datetimeFields[shard] = this.datetimeFields[base]; @@ -9757,6 +9793,7 @@ export class SqlDriver implements IDataDriver { const jsonCols: string[] = []; const booleanCols: string[] = []; const numericCols: string[] = []; + const numericValueCols: string[] = []; const dateCols: string[] = []; const datetimeCols: string[] = []; const timeCols: string[] = []; @@ -9769,6 +9806,8 @@ export class SqlDriver implements IDataDriver { if (this.isJsonField(type, field)) jsonCols.push(name); if (type === 'boolean' || type === 'toggle') booleanCols.push(name); if (NUMERIC_SCALAR_TYPES.has(type) && !field.multiple) numericCols.push(name); + // [#16318] The authorable half only — see {@link numericValueFields}. + if (NUMERIC_VALUE_TYPES.has(type) && !field.multiple) numericValueCols.push(name); if (type === 'date') dateCols.push(name); if (type === 'datetime') datetimeCols.push(name); if (type === 'time') timeCols.push(name); @@ -9781,6 +9820,7 @@ export class SqlDriver implements IDataDriver { this.jsonFields[key] = jsonCols; this.booleanFields[key] = booleanCols; this.numericFields[key] = numericCols; + this.numericValueFields[key] = numericValueCols; this.autoNumberFields[key] = autoNumberCols; this.tenantFieldByTable[key] = tenantField; if (dateCols.length) this.dateFields[key] = new Set(dateCols); @@ -9827,6 +9867,7 @@ export class SqlDriver implements IDataDriver { const jsonCols: string[] = []; const booleanCols: string[] = []; const numericCols: string[] = []; + const numericValueCols: string[] = []; const autoNumberCols: Array<{ name: string; format: string; tokens: AutonumberToken[]; tenantField: string | null }> = []; // Tenant-isolation column: explicit tenancy opt-out → declared field → // implicit `organization_id`. See {@link computeAndRecordTenantField} @@ -9850,6 +9891,10 @@ export class SqlDriver implements IDataDriver { if (NUMERIC_SCALAR_TYPES.has(type) && !field.multiple) { numericCols.push(name); } + // [#16318] The authorable half only — see {@link numericValueFields}. + if (NUMERIC_VALUE_TYPES.has(type) && !field.multiple) { + numericValueCols.push(name); + } if (type === 'date') { (this.dateFields[tableName] ??= new Set()).add(name); } @@ -9872,6 +9917,7 @@ export class SqlDriver implements IDataDriver { this.jsonFields[tableName] = jsonCols; this.booleanFields[tableName] = booleanCols; this.numericFields[tableName] = numericCols; + this.numericValueFields[tableName] = numericValueCols; this.autoNumberFields[tableName] = autoNumberCols; this.tenantFieldByTable[tableName] = tenantField; // [#11067] The declared shape's answer to "does this table carry @@ -13121,8 +13167,11 @@ export class SqlDriver implements IDataDriver { * `formatOutput` gates its row reads that way (#11782; SQLite-only before, * which is how a declared boolean answered `1`/`0` on MySQL). Postgres * stores a real `boolean` node-pg parses, so there the stored form already - * IS the presented form. The numeric repair stays SQLite-only: it exists - * for legacy TEXT-affinity columns, which no other dialect has. + * IS the presented form. The numeric repair ran SQLite-only on the premise + * that string-valued numerics exist only for legacy TEXT-affinity columns; + * #16318 falsified that premise by moving the numeric family to an exact + * decimal, which node-postgres and mysql2 both hand back as a string, so it + * runs on every dialect now. */ protected readPresentationKind( table: string | null | undefined, @@ -13135,7 +13184,11 @@ export class SqlDriver implements IDataDriver { if ((this.isSqlite || this.isMysql) && this.booleanFields[table]?.includes(field)) { return 'boolean'; } - if (!this.isSqlite) return null; + // [#16318] Every dialect, for the reason `formatOutput`'s own numeric pass + // records: an exact-decimal column is handed back as a STRING by + // node-postgres and by mysql2, so `aggregate()` / `distinct()` would present + // a declared numeric field as a string on exactly the dialects `find()` now + // presents it as a number. One class, one answer, on every door. if (this.numericFields[table]?.includes(field)) return 'number'; return null; } @@ -15934,8 +15987,12 @@ export class SqlDriver implements IDataDriver { // Virtual — `createColumn` returns without emitting anything. case 'formula': return null; - // The non-string primitives: INTEGER / REAL / BOOLEAN / DATE / DATETIME / - // TIME columns. None of them is sized from metadata and none is a varchar. + // The non-string primitives: INTEGER / REAL / DECIMAL / BOOLEAN / DATE / + // DATETIME / TIME columns. None of them is a varchar, so none is sized + // from `maxLength` — which is the only question this mirror answers. + // ⚠️ Since #16318 the numeric members ARE sized, by `numericColumnFor`, + // but from the field's TYPE and not from any declaration; `null` here + // stays the correct answer to the question actually asked. case 'integer': case 'int': case 'float': @@ -16413,21 +16470,67 @@ export class SqlDriver implements IDataDriver { case 'int': col = table.integer(name); break; + // `float` is a DRIVER-SIDE ALIAS, not a `FieldType`: there is no + // `Field.float` builder and `NUMERIC_VALUE_TYPES` does not carry it, so + // #16318's table has no opinion about it and it keeps the column it has + // always had. case 'float': - case 'number': - case 'currency': - case 'percent': + col = table.float(name); + break; + // [#16318] The seven members of `NUMERIC_VALUE_TYPES` take the physical + // representation `packages/spec` states for them ({@link + // numericColumnFor}) — the same table both `os generate migration` + // formats read, so one declaration can no longer produce three different + // columns (measured on live PostgreSQL 16.13: this arm's `real`, the sql + // format's `numeric(18,2)`/`numeric(5,2)`, and the typescript format's + // `numeric(8,2)`). The spec module carries the measurements, the ruling + // and the residual bound; ⛔ do not restate its numbers here. + // + // What this arm still owes its reader is the SQLite half, because that is + // why three of these types were put in a float arm in the first place: + // // `rating`/`slider`/`progress` are authored as numeric scalars (a star // count, a slider position, a percent-of-completion). Without an explicit // case they fell to `default → table.string`, giving the column TEXT // affinity so SQLite coerced the written number to a string ('4' not 4) — // a silent type-fidelity leak the value-loss tests didn't catch. REAL // affinity round-trips them as JS numbers (#field-zoo). + // + // That leak stays defeated, and MEASURED rather than argued: knex + // compiles `table.decimal(name, p, s)` and `table.float(name)` to the + // IDENTICAL `float` column on SQLite + // (`ColumnCompiler_SQLite3.prototype.decimal` is the literal `'float'`), + // so the six exact-decimal members emit byte-identical SQLite DDL to what + // this arm emitted before and keep REAL affinity. `rating` moves to + // INTEGER affinity, where SQLite stores `4` as an integer and still + // accepts `4.5` as a REAL — it refuses no fractional value — so nothing + // this dialect accepts today stops being accepted. + // + // ⚠️ The read path is what makes the move safe on the server dialects: + // node-postgres parses `numeric` to a STRING and `real` to a number, and + // it is `NUMERIC_SCALAR_TYPES`' existing `numericFields` coercion — + // already registered for all seven of these types — that turns it back + // into a JS number on the way out. + case 'number': + case 'currency': + case 'percent': case 'rating': case 'slider': case 'progress': - col = table.float(name); + case 'summary': { + const numeric = numericColumnFor(type); + // ⛔ Not `?? table.float(name)`: an undefined answer for a type named + // in these very case labels would mean the labels and + // `NUMERIC_VALUE_TYPES` have parted, and a silent fallback is exactly + // the drift #16318 exists to close. The spec-side pin + // (`numeric-column-representation.test.ts`) holds the two equal, and + // `NUMERIC_VALUE_TYPES` is the single membership authority both read. + col = + numeric === undefined || numeric.kind === 'integer' + ? table.integer(name) + : table.decimal(name, numeric.precision, numeric.scale); break; + } // `toggle` is a boolean rendered as a switch. Same leak as above (TEXT // affinity stored '1'); a boolean column gives NUMERIC affinity and the // `booleanFields` read-coercion below converts the stored 1/0 back to a @@ -16522,9 +16625,6 @@ export class SqlDriver implements IDataDriver { // types.mdx` has told authors since 2026-07-30. col = table.string(name); break; - case 'summary': - col = table.float(name); - break; case 'auto_number': case 'autonumber': // ⛔ Also out of #11431's scope, for a different reason than `lookup` @@ -17018,23 +17118,64 @@ export class SqlDriver implements IDataDriver { } } - // Numeric scalars stored on a legacy TEXT-affinity column come back as - // strings ('4'); coerce numeric-looking strings back to numbers so the - // declared type wins regardless of when the column was created. Only - // touch strings — a fresh REAL/INTEGER column already yields a number, - // and a genuinely non-numeric value (junk legacy data) is left intact - // rather than turned into NaN. See NUMERIC_SCALAR_TYPES. - const numericFields = this.numericFields[object]; - if (numericFields && numericFields.length > 0) { - for (const field of numericFields) { - const v = data[field]; - if (typeof v === 'string' && v.trim() !== '') { - const n = Number(v); - if (!Number.isNaN(n)) data[field] = n; - } + } + + // Numeric scalars handed back as STRINGS are coerced to numbers, on EVERY + // dialect, so the declared type wins regardless of which dialect stored the + // value or when the column was created. Only strings are touched — a + // REAL/INTEGER column already yields a number — and a genuinely non-numeric + // value (junk legacy data) is left intact rather than turned into NaN. + // See NUMERIC_SCALAR_TYPES. + // + // ⚠️ [#16318] This pass was SQLite-only until the numeric family moved to an + // exact-decimal column, on the stated premise that string-valued numerics + // "exist for legacy TEXT-affinity columns, which no other dialect has". + // That premise is now false and it is THIS change that falsified it, so it + // is corrected here rather than left as a fossil: measured on live + // PostgreSQL 16.13, node-postgres parses `real` to a JS `number` and + // `numeric` to a STRING (`1234567.89` arrives as + // `'1234567.890000000000000000000000000000'`), and mysql2 does the same for + // `DECIMAL`. Without this line every `number` / `currency` / `percent` / + // `slider` / `progress` / `summary` field would start reading back as a + // string on the two server dialects — a wire-contract break, since + // `valueSchemaFor` gives the whole class `z.number().finite()`. + // + // Two SQLite readings stay exactly as they were: the legacy TEXT-affinity + // repair this pass was written for, and a fresh column, which knex declares + // `float` for both the old float arm and the new decimal one. + // + // ⚠️ [#16318] The registry is chosen PER DIALECT, and the narrowing is the + // "new tables only" bound, not a taste. `numericFields` carries the + // driver-internal aliases `integer` / `int` / `float`, which is how an + // EXTERNAL, introspected table's columns reach this driver. A PostgreSQL + // `bigint` is handed back by node-postgres as a STRING precisely because it + // does not fit a JS double, so running it through `Number()` would silently + // round it above 2^53 — on a table this change never created. SQLite is the + // one dialect where the wider set is right, because there the pass exists + // for legacy TEXT-affinity columns of exactly those alias types. + // + // ⚠️ The repair is bounded by the wire contract it restores, and that bound + // is binary64: `valueSchemaFor` gives this whole class `z.number().finite()` + // (ADR-0104 D1), so a `find()` result is a JS double however exact the + // COLUMN is. Measured: a value the driver itself wrote from a JS number + // round-trips exactly ('1234567.890000000000000000000000000000' → 1234567.89), + // because the shortest representation is what was stored; a value that was + // never a double does not ('1234567890123456.123' → 1234567890123456, + // 2^53+1 → 2^53). ⇒ the exactness this change buys is exact-column-through- + // a-double: SQL-side writers, `summary` roll-ups computed in SQL and any + // magnitude at or above 2^53 are bounded by the read seam, not by the + // column. Widening that is a wire-contract change and is NOT in #16318. + const numericFields = this.isSqlite + ? this.numericFields[object] + : this.numericValueFields[object]; + if (numericFields && numericFields.length > 0) { + for (const field of numericFields) { + const v = data[field]; + if (typeof v === 'string' && v.trim() !== '') { + const n = Number(v); + if (!Number.isNaN(n)) data[field] = n; } } - } // [ADR-0053 D-F1] (#13973) — the two instant classes present as ONE shape diff --git a/packages/services/service-analytics/src/measure-result-type.ts b/packages/services/service-analytics/src/measure-result-type.ts index 806b5a3614..1ac2563cae 100644 --- a/packages/services/service-analytics/src/measure-result-type.ts +++ b/packages/services/service-analytics/src/measure-result-type.ts @@ -158,13 +158,19 @@ import { * * Both shipped statements agree: `summary` is a member of the spec's * `NUMERIC_VALUE_TYPES` (so `valueSchemaFor` answers `z.number().finite()`) - * and `driver-sql`'s DDL answers `col = table.float(name)`. The producer's - * `number` is therefore the CORRECT word and no correction applies. That a + * and `driver-sql`'s DDL answers with a numeric column. Since #16318 that + * column is the exact decimal `NUMERIC_COLUMN_REPRESENTATION` states — `col = + * table.decimal(name, 65, 30)` on a NEW table, where it was `col = + * table.float(name)` before and still is on every table created earlier. The + * producer's `number` is the CORRECT word either way and no correction + * applies; ⛔ nothing in this rule reads the column's precision. That a * roll-up may declare `summaryOperations.function: 'min'` over a non-numeric * child field — which `aggregateSummaryValue` returns verbatim, into that - * float column — is a defect one layer down in the same family; it is filed, + * numeric column — is a defect one layer down in the same family; it is filed, * and it is a statement about `summary`'s own storage, not about what this - * rule should say for the declared type. + * rule should say for the declared type. ⚠️ The exact column REFUSES that + * verbatim text where the float column refused it too, so the retype neither + * creates nor closes it. * * ## What this rule deliberately cannot see: `multiple` * diff --git a/packages/spec/api-surface/data.json b/packages/spec/api-surface/data.json index ad8388155c..9acb85ea9c 100644 --- a/packages/spec/api-surface/data.json +++ b/packages/spec/api-surface/data.json @@ -413,6 +413,9 @@ "MysqlConfigSchema (const)", "NON_TEXT_STORED_VALUE_TYPES (const)", "NOW_DEFAULT_LEGAL_TYPES (const)", + "NUMERIC_COLUMN_PRECISION (const)", + "NUMERIC_COLUMN_REPRESENTATION (const)", + "NUMERIC_COLUMN_SCALE (const)", "NUMERIC_VALUE_TYPES (const)", "NoSQLDataTypeMapping (type)", "NoSQLDataTypeMappingSchema (const)", @@ -434,6 +437,7 @@ "NoSQLTransactionOptionsSchema (const)", "NormalizedFilter (type)", "NormalizedFilterSchema (const)", + "NumericColumnRepresentation (type)", "OBJECT_KEY_GUIDANCE (const)", "OWNER_FIELD_DEF (const)", "OWNING_BUSINESS_UNIT_FIELD_DEF (const)", @@ -776,6 +780,7 @@ "missingFieldValues (function)", "nextUtcCalendarDay (function)", "normalizeFilterComparandTypes (function)", + "numericColumnFor (function)", "objectForm (const)", "objectTitleCompleteness (function)", "parseAutonumberFormat (function)", diff --git a/packages/spec/export-origins/data.json b/packages/spec/export-origins/data.json index 198229b0da..54556012ae 100644 --- a/packages/spec/export-origins/data.json +++ b/packages/spec/export-origins/data.json @@ -403,6 +403,9 @@ "MysqlConfigSchema": "src/data/driver/mysql.zod.ts#MysqlConfigSchema (const)", "NON_TEXT_STORED_VALUE_TYPES": "src/data/field-value.zod.ts#NON_TEXT_STORED_VALUE_TYPES (const)", "NOW_DEFAULT_LEGAL_TYPES": "src/data/default-value-shape.ts#NOW_DEFAULT_LEGAL_TYPES (const)", + "NUMERIC_COLUMN_PRECISION": "src/data/numeric-column-representation.ts#NUMERIC_COLUMN_PRECISION (const)", + "NUMERIC_COLUMN_REPRESENTATION": "src/data/numeric-column-representation.ts#NUMERIC_COLUMN_REPRESENTATION (const)", + "NUMERIC_COLUMN_SCALE": "src/data/numeric-column-representation.ts#NUMERIC_COLUMN_SCALE (const)", "NUMERIC_VALUE_TYPES": "src/data/field-value.zod.ts#NUMERIC_VALUE_TYPES (const)", "NoSQLDataTypeMapping": "src/data/driver-nosql.zod.ts#NoSQLDataTypeMapping (type)", "NoSQLDataTypeMappingSchema": "src/data/driver-nosql.zod.ts#NoSQLDataTypeMappingSchema (const)", @@ -424,6 +427,7 @@ "NoSQLTransactionOptionsSchema": "src/data/driver-nosql.zod.ts#NoSQLTransactionOptionsSchema (const)", "NormalizedFilter": "src/data/filter.zod.ts#NormalizedFilter (type)", "NormalizedFilterSchema": "src/data/filter.zod.ts#NormalizedFilterSchema (const)", + "NumericColumnRepresentation": "src/data/numeric-column-representation.ts#NumericColumnRepresentation (type)", "OBJECT_KEY_GUIDANCE": "src/data/authoring-key-lint.ts#OBJECT_KEY_GUIDANCE (const)", "OWNER_FIELD_DEF": "src/data/injected-system-column-provenance.ts#OWNER_FIELD_DEF (const)", "OWNING_BUSINESS_UNIT_FIELD_DEF": "src/data/injected-system-column-provenance.ts#OWNING_BUSINESS_UNIT_FIELD_DEF (const)", @@ -763,6 +767,7 @@ "missingFieldValues": "src/data/autonumber-format.ts#missingFieldValues (function)", "nextUtcCalendarDay": "src/data/calendar-day.ts#nextUtcCalendarDay (function)", "normalizeFilterComparandTypes": "src/data/filter-comparand-type.ts#normalizeFilterComparandTypes (function)", + "numericColumnFor": "src/data/numeric-column-representation.ts#numericColumnFor (function)", "objectForm": "src/data/object.form.ts#objectForm (const)", "objectTitleCompleteness": "src/data/display-name.ts#objectTitleCompleteness (function)", "parseAutonumberFormat": "src/data/autonumber-format.ts#parseAutonumberFormat (function)", diff --git a/packages/spec/src/api/sortability.zod.ts b/packages/spec/src/api/sortability.zod.ts index a454a928a4..ff787af114 100644 --- a/packages/spec/src/api/sortability.zod.ts +++ b/packages/spec/src/api/sortability.zod.ts @@ -61,10 +61,14 @@ * ## Considered and deliberately NOT members * * - `summary` / `autonumber` — the other two `COMPUTED_VALUE_TYPES`. They sort - * CORRECTLY (`summary` is an engine-maintained `table.float`, `autonumber` - * an engine-assigned `table.string`; measured on #6924), which is exactly - * why virtuality is judged by the storage predicate and never by the write - * contract — widening would refuse the two types that work. + * CORRECTLY (`summary` is an engine-maintained numeric column — `table.float` + * when #6924 measured it, an exact `table.decimal` on new tables since + * #16318's stated representation — and `autonumber` an engine-assigned + * `table.string`), which is exactly why virtuality is judged by the storage + * predicate and never by the write contract — widening would refuse the two + * types that work. ⚠️ The column TYPE is not what makes them sortable — + * having a PROVISIONED column is — which is why #16318's retype of the + * numeric family moved nothing in this projection. * - `encrypted` / `secret` / `json` / `vector` and the other heavy or masked * types — every one has a stored column, neither door refuses an ORDER BY * over one, and the drivers execute it. Marking them unsortable here would diff --git a/packages/spec/src/data/index.ts b/packages/spec/src/data/index.ts index 8f83fe5b2e..0e5547e7b8 100644 --- a/packages/spec/src/data/index.ts +++ b/packages/spec/src/data/index.ts @@ -219,6 +219,12 @@ export * from './aggregate-field-type-compatibility'; // from field metadata so renderers never guess it from the value's magnitude. export * from './percent-scale'; +// The NUMERIC column family's physical representation (#16318) — the single +// per-field-type table `SqlDriver.createColumn` and both `os generate +// migration` formats read, so one declaration cannot produce three different +// columns. +export * from './numeric-column-representation'; + // Record display-name contract (ADR-0079) — title eligibility, primary-field // resolution/derivation, record display-name rendering, primary provisioning, // and title-completeness classification. Shared by authoring, display diff --git a/packages/spec/src/data/numeric-column-representation.test.ts b/packages/spec/src/data/numeric-column-representation.test.ts new file mode 100644 index 0000000000..692d1d1de8 --- /dev/null +++ b/packages/spec/src/data/numeric-column-representation.test.ts @@ -0,0 +1,70 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The #16318 pin: the physical-representation table and `NUMERIC_VALUE_TYPES` + * are held EQUAL, in both directions. + * + * Why a pin and not a type: `NUMERIC_VALUE_TYPES` is a `ReadonlySet`, + * so no `satisfies` can express "exactly these keys". Without this test a + * field type joining the numeric class would resolve to `undefined` here and + * every producer would quietly keep its own old guess — which is the divergence + * the card measured (driver `real`, sql format `numeric(18,2)`, typescript + * format `numeric(8,2)`, on the same declaration). + */ + +import { describe, it, expect } from 'vitest'; +import { NUMERIC_VALUE_TYPES } from './field-value.zod'; +import { + NUMERIC_COLUMN_PRECISION, + NUMERIC_COLUMN_SCALE, + NUMERIC_COLUMN_REPRESENTATION, + numericColumnFor, +} from './numeric-column-representation'; + +describe('#16318 — the numeric physical-representation table', () => { + it('names every member of NUMERIC_VALUE_TYPES and nothing else', () => { + expect(Object.keys(NUMERIC_COLUMN_REPRESENTATION).sort()).toEqual([...NUMERIC_VALUE_TYPES].sort()); + }); + + it('answers for every member of the class', () => { + for (const type of NUMERIC_VALUE_TYPES) { + expect(numericColumnFor(type), type).toBeDefined(); + } + }); + + it('has NO opinion about a type outside the class', () => { + // The driver's internal SQL aliases, a character type, and the two + // spellings a malformed declaration can reach the resolver with. + for (const type of ['float', 'integer', 'int', 'text', 'boolean', '', undefined]) { + expect(numericColumnFor(type as string | undefined), String(type)).toBeUndefined(); + } + }); + + it('gives rating an INTEGER column and the other six an exact decimal', () => { + expect(numericColumnFor('rating')).toEqual({ kind: 'integer' }); + for (const type of ['number', 'currency', 'percent', 'slider', 'progress', 'summary']) { + expect(numericColumnFor(type), type).toEqual({ + kind: 'exact', + precision: NUMERIC_COLUMN_PRECISION, + scale: NUMERIC_COLUMN_SCALE, + }); + } + }); + + /** + * The scale is the whole point of the card, so it is pinned as a NUMBER and + * with the property that number was chosen for: a `percent` stores 33.333% + * as the fraction `0.33333` (`percentScaleOf`), which needs five decimal + * places, and the two shapes the producers used to emit have two. + */ + it('pins the portable dialect maxima, and a scale wide enough for the card\'s own value', () => { + expect(NUMERIC_COLUMN_PRECISION).toBe(65); + expect(NUMERIC_COLUMN_SCALE).toBe(30); + expect(NUMERIC_COLUMN_SCALE).toBeGreaterThan(2); + expect(NUMERIC_COLUMN_SCALE).toBeGreaterThanOrEqual('0.33333'.split('.')[1].length); + // MySQL's own caps, which is where both numbers come from. + expect(NUMERIC_COLUMN_SCALE).toBeLessThanOrEqual(30); + expect(NUMERIC_COLUMN_PRECISION).toBeLessThanOrEqual(65); + expect(NUMERIC_COLUMN_PRECISION).toBeGreaterThan(NUMERIC_COLUMN_SCALE); + }); +}); diff --git a/packages/spec/src/data/numeric-column-representation.ts b/packages/spec/src/data/numeric-column-representation.ts new file mode 100644 index 0000000000..437aee93a0 --- /dev/null +++ b/packages/spec/src/data/numeric-column-representation.ts @@ -0,0 +1,252 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The NUMERIC column family's PHYSICAL REPRESENTATION — one explicit, + * per-field-type table that every producer of DDL reads (#16318). + * + * Ruled C ∩ ④ (director seat, decision batch #86, 2026-09-08). Quoted, not + * translated: + * + * > One explicit per-field-type physical-representation table lives in + * > `packages/spec` (the protocol is the baseline) and both producers — + * > `SqlDriver.createColumn` and `os generate migration` (sql + typescript + * > formats) — read it … **New tables only**: no migration of existing + * > columns (「不考虑现有数据」); SQLite affinity consequences stated per type. + * + * ## The divergence this closes — measured, not argued + * + * One object, seven plain numeric declarations, three producers, driven into + * live PostgreSQL 16.13 and read back out of `information_schema.columns` + * with `numeric_precision` / `numeric_scale` — the half a bare `data_type` + * read hides: + * + * ``` + * driver sql gen ts gen + * number real numeric(18,2) numeric(8,2) + * currency real numeric(18,2) numeric(8,2) + * percent real numeric(5,2) numeric(8,2) + * slider real numeric(18,2) numeric(8,2) + * summary real numeric(18,2) numeric(8,2) + * progress real numeric(5,2) numeric(8,2) + * rating real integer integer + * ``` + * + * 7 of 7 diverge, and six of them THREE ways rather than the two the report + * named: `table.decimal(name)` with no arguments is knex's `decimal(8, 2)`, + * so the two halves of one command never agreed with each other either. A + * control family already unified (#16091: `text` / `email` / `boolean` / + * `date`) came back 0-of-4 divergent in the same run, so AGREE is a reading + * the instrument can produce. + * + * ## Why a NARROW scale could not stay, and what the loss actually is + * + * The report's reason was an INFERENCE off the DDL literal — that a + * `DECIMAL(5,2)` column truncates a legitimate 33.333 to 33.33. Executed on + * PostgreSQL 16.13, the direction is wrong and the substance holds: the + * column ROUNDS half-up (33.336 arrives as 33.34), it does not truncate. The + * loss is silent either way. + * + * Nothing upstream prevents it. Measured at the write seam: a `percent` / + * `currency` / `number` field that declares no `scale` ACCEPTS 0.33333 and + * 1234567.89 unchanged (4 of 4), while the same seam REFUSES both the moment + * the field declares `scale: 2` (2 of 2 controls fired). So there is no + * upstream rounding and no upstream validation to fall back on — for a field + * with no declared `scale` the COLUMN is the only thing deciding, and a + * narrow one silently alters data. `record-validator.ts` states the platform's + * position on exactly that (#7501, maintainer ruling 2026-08-11): an + * over-scale value "is refused the way an out-of-range one is; silent + * rounding is silently altering data". + * + * ⚠️ `summary` has no seam at all — it is platform-computed, and + * `validateRecord`'s type door excludes it — so for that member the column is + * the ONLY guard. + * + * ## Where the two numbers come from — both are dialect maxima, not taste + * + * Every candidate column was driven against a nine-value corpus on live + * PostgreSQL 16.13, written and read back through the driver's own pg type + * parsing, with `12.5` as the firing control (a dyadic rational every + * candidate holds exactly — it came back EXACT from all five, 0 of 5 lost): + * + * ``` + * real 3/9 altered <- the driver today + * numeric(8,2) 9/9 altered <- the typescript format today + * numeric(18,2) 7/9 altered <- the sql format today + * numeric(38,17) 2/9 altered + * numeric(65,30) 0/9 altered <- this table + * ``` + * + * `real` is IEEE-754 binary32 and its 3 are the ones that matter most: + * `1234567.89` reads back `1234567.9` and `Number.MAX_SAFE_INTEGER` reads + * back `9007199000000000`. That is the money-fidelity defect the report + * named, in a reading rather than an argument. + * + * {@link NUMERIC_COLUMN_SCALE} is 30 and {@link NUMERIC_COLUMN_PRECISION} is + * 65 because those are MySQL's documented `DECIMAL` maxima — the binding + * constraint among the dialects this platform speaks, PostgreSQL's ceiling + * being 1000 digits and SQLite having none. Taking the maximum is what makes + * the residual bound as far out as any portable exact-decimal column can put + * it; ⛔ neither number is chosen for how it reads. + * + * ⚠️ An unconstrained `numeric` would be the honest shape on PostgreSQL and it + * is NOT portable: measured through knex's own compilers, `decimal(name, + * null)` compiles to `decimal` on `pg`, to `float` on `better-sqlite3`, and + * THROWS on `mysql2` ("Specifying no precision on decimal columns is not + * supported"). A stated pair is the only spelling all three accept, which is + * what the ruling asked for. + * + * ## The residual bound, stated rather than assumed + * + * An exact-decimal column is bounded where a float is not, so this table is + * not lossless in every direction. Magnitudes below 1e-30 round to zero and + * magnitudes at or above 1e35 are REFUSED, where today's `real` keeps about + * seven significant digits out to ~1e38. Two things make that the right + * trade: a refusal is loud and a silent rounding is not, and the sql format's + * `numeric(18,2)` already refuses everything at or above 1e16 today. It is a + * bound, and it is stated here so no reader has to rediscover it. + * + * ## SQLite, per type — the constraint the report raised, answered + * + * `SqlDriver.createColumn`'s float arm records why `rating`/`slider`/ + * `progress` are in it: without an explicit case they fell to `table.string`, + * the column took TEXT affinity, and SQLite stored `'4'` rather than `4`. + * Measured on knex 3.3.0 / better-sqlite3, compiled DDL and live storage + * class: + * + * ``` + * table.float(c) -> float real:4 real:4.5 real:33.333 + * table.decimal(c, 65, 30) -> float real:4 real:4.5 real:33.333 + * table.integer(c) -> integer integer:4 real:4.5 real:33.333 + * table.string(c) -> varchar(255) text:4.0 <- the fossil's leak + * ``` + * + * Two per-type consequences follow, and neither is assumed: + * + * - The six exact-decimal members emit BYTE-IDENTICAL SQLite DDL to the + * float arm they leave — `ColumnCompiler_SQLite3.prototype.decimal` is the + * literal `'float'`, the same string `floating` resolves to — so they keep + * REAL affinity and the fossil's leak stays defeated. SQLite applies no + * precision and no scale, so the exactness this table buys is a + * PostgreSQL/MySQL property; SQLite behaves exactly as it does today. + * - `rating` moves to INTEGER affinity. `4` is then stored as the integer + * `4` rather than the real `4.0`, and SQLite still accepts `4.5` as a REAL + * — it refuses no fractional value — so nothing this dialect accepts today + * stops being accepted. The refusal `rating` gains is a + * PostgreSQL/MySQL-only effect. + * + * A `table.string` control in the same run still compiled to `varchar(255)` + * and still stored `text:4.0`, so "identical" above is a discriminating + * reading and not a constant. + * + * ## `rating` — integer, and the half-star need was looked for + * + * The ruling made `rating` integer "unless the executor measures a half-star + * need". There is no capability to measure: `Field.rating` takes a star COUNT + * and the spec declares no half-star key for it. A field that genuinely wants + * fractional stars is a `slider`, which is in the exact-decimal set above. + * + * ## Scope — NEW COLUMNS ONLY + * + * ⛔ This table decides what a NEW column is created as, and nothing else. It + * retypes no existing column (schema sync is additive and never alters a + * column's type in place), it plans no migration, and no drift finding reads + * it. A deployment created before this table keeps its `real` columns, keeps + * their values, and keeps reading them back as JS numbers through + * `NUMERIC_SCALAR_TYPES`' read coercion — which is also what makes the new + * columns read back as numbers, since node-postgres parses `numeric` to a + * STRING and `real` to a number. + */ + +import { NUMERIC_VALUE_TYPES } from './field-value.zod'; + +/** + * Total digits for every exact-decimal column this table produces — MySQL's + * documented `DECIMAL` maximum, and therefore the portable one. See the + * provenance block above; ⛔ do not "tidy" it to a rounder number. + */ +export const NUMERIC_COLUMN_PRECISION = 65; + +/** + * Decimal places for every exact-decimal column this table produces — MySQL's + * documented maximum `DECIMAL` scale, and the only scale measured to lose + * nothing on the nine-value corpus above. + */ +export const NUMERIC_COLUMN_SCALE = 30; + +/** + * What a numeric field's column IS, kept as NAMED answers rather than a bare + * pair — the same reason `generate.ts`'s `VarcharAnswer` is three answers and + * not a number. `integer` is not "an exact decimal with scale 0": it is a + * different column type with a different refusal, and on SQLite a different + * affinity. + */ +export type NumericColumnRepresentation = + | { readonly kind: 'integer' } + | { readonly kind: 'exact'; readonly precision: number; readonly scale: number }; + +const EXACT: NumericColumnRepresentation = { + kind: 'exact', + precision: NUMERIC_COLUMN_PRECISION, + scale: NUMERIC_COLUMN_SCALE, +}; + +/** + * The per-type table itself. + * + * Keyed on every member of `NUMERIC_VALUE_TYPES` and nothing else. The + * equality is PINNED rather than typed — `NUMERIC_VALUE_TYPES` is a + * `ReadonlySet`, so no `satisfies` can express it — and + * `numeric-column-representation.test.ts` fails in BOTH directions: a type + * joining that class with no entry here, and an entry here naming a type that + * left it. Without the pin a new member would resolve to `undefined` and each + * producer would quietly keep its own old guess, which is the exact shape + * #16318 exists to close. + * + * ⚠️ The driver's internal SQL aliases (`float`, `integer`, `int`) are NOT + * members of this class and deliberately have no entry: they are not + * `FieldType`s, nothing authorable produces them, and they keep the columns + * they have always had. + */ +export const NUMERIC_COLUMN_REPRESENTATION: Readonly> = { + // Open-range quantities. + number: EXACT, + // Money — the one member where the loss is a correctness question rather + // than a display one, and the reason binary32 could not stay. ⛔ Not a + // blanket `18,2`: the platform's own CLDR table carries 0-digit currencies + // (JPY, KRW, ...) and 3-digit ones (BHD, KWD, ...), and its currency-code + // schema deliberately fails OPEN for crypto and custom codes, which carry + // more (see `currency-fraction-digits.ts`). A money column that fixes two + // decimals is wrong for a set the platform declines to close. + currency: EXACT, + // ⚠️ A `percent` stores a 0-1 FRACTION unless the field declares `max > 1` + // (`percentScaleOf`), so the legitimate value the ruling names — 33.333% — + // reaches the column as `0.33333`, where `numeric(5,2)` rounded it to + // `0.33`: 33% for 33.333%. Both storage scales are held exactly here. + percent: EXACT, + slider: EXACT, + // The same 0-100 quantity as `percent`, which is why it took `percent`'s + // NARROW shape in the sql format. It keeps sharing `percent`'s answer; the + // shared answer is now the wide one. + progress: EXACT, + // A platform-computed roll-up: the SUM of child values, so it needs at least + // what its children have, and it is the member with no write seam to refuse + // an over-scale value on its behalf. + summary: EXACT, + // A star count. See the half-star block above. + rating: { kind: 'integer' }, +}; + +/** + * The column a numeric field type takes, or `undefined` when the type is not a + * member of this family at all — this table has NO opinion about those, and a + * caller that gets `undefined` keeps whatever answer it already had. + * + * ⛔ Callers must not spell a fallback column for a member of this family: an + * `undefined` for one would mean the caller's case labels and + * `NUMERIC_VALUE_TYPES` have parted, and a silent default is the drift this + * table closes. The pin holds the two equal. + */ +export function numericColumnFor(type: string | undefined): NumericColumnRepresentation | undefined { + if (typeof type !== 'string' || !NUMERIC_VALUE_TYPES.has(type)) return undefined; + return NUMERIC_COLUMN_REPRESENTATION[type]; +}