diff --git a/.changeset/introspection-contract-withdraw-indexes-widen-defaults.md b/.changeset/introspection-contract-withdraw-indexes-widen-defaults.md new file mode 100644 index 0000000000..5d10c236df --- /dev/null +++ b/.changeset/introspection-contract-withdraw-indexes-widen-defaults.md @@ -0,0 +1,40 @@ +--- +"@objectstack/spec": patch +"@objectstack/driver-sql": patch +"@objectstack/objectql": patch +--- + +Withdraw the never-honored `IntrospectedTable.indexes` promise and widen two +introspection declarations to the measured emitted types (#11122, maintainer +ruling 2026-08-23, option B — 「其他同意你的意见」). + +The spec's introspection contract (`schema-diff-service.ts`) declared +`indexes: IntrospectedIndex[]` as REQUIRED, yet no producer has ever emitted +it — a consumer typed against the promise read `undefined` with no compiler +complaint. It also declared `defaultValue?: string` while the in-tree SQL +driver passes `knex.columnInfo().defaultValue` through raw (measured on live +SQLite: `null` for a column with no default, dialect-quoted strings such as +`'abc'` otherwise; other producers report native values such as `true`). + +- `IntrospectedTable.indexes` is now **optional**, and absence is meaningful: + an absent key means the producer did not read indexes; an empty array is a + positive claim the table HAS none. Producers that did not look must omit + the key rather than emit `[]`. Wiring the index read into + `introspectSchema()` is explicitly NOT part of this change. +- `IntrospectedColumn.defaultValue` is now `unknown` — consumers narrow + before use instead of trusting a string promise no producer kept. +- The SQL layer's extra `maxLength` fact (driver-sql / objectql + `IntrospectedColumn`, driver-sql `PhysicalColumn`) widens from `number` to + `number | string` — SQLite reports the string `"255"` where other dialects + report a number. + +With the spec now telling the truth, the deliberate `Omit` workarounds in +`@objectstack/driver-sql` and `@objectstack/objectql` (which carved +`defaultValue` and `indexes` out of the spec types to keep the divergence +visible) are retired: both packages' introspection types now extend the spec +contract directly. + +Consumers that read `table.indexes` must guard for absence (none exist +in-tree — the requirement was never honored, so today's readers would have +crashed on `undefined` anyway); consumers of `defaultValue` must narrow from +`unknown` before string operations. diff --git a/packages/drivers/driver-sql/src/schema-drift.ts b/packages/drivers/driver-sql/src/schema-drift.ts index 3a16c6f5e2..26c28db693 100644 --- a/packages/drivers/driver-sql/src/schema-drift.ts +++ b/packages/drivers/driver-sql/src/schema-drift.ts @@ -305,7 +305,12 @@ export interface PhysicalColumn { name: string; type: string; nullable: boolean; - maxLength?: number; + /** + * Raw as knex `columnInfo()` reports it — a number on some dialects, a + * STRING on SQLite (measured: `"255"`). The varchar differ below narrows + * via `typeof` before comparing, which is the pattern for any new reader. + */ + maxLength?: number | string; /** * The column's raw DEFAULT as the dialect reports it (knex `columnInfo`), or * `null`/`undefined` when it has none. Dialect-decorated — SQLite and Postgres diff --git a/packages/drivers/driver-sql/src/sql-driver-introspection-spec-contract.test.ts b/packages/drivers/driver-sql/src/sql-driver-introspection-spec-contract.test.ts index 7529052b04..03cb6abb4f 100644 --- a/packages/drivers/driver-sql/src/sql-driver-introspection-spec-contract.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-introspection-spec-contract.test.ts @@ -23,6 +23,7 @@ */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import type { IntrospectedSchema as SpecIntrospectedSchema } from '@objectstack/spec/contracts'; import { SqlDriver } from '../src/index.js'; describe('SqlDriver.introspectSchema emits the spec introspection contract', () => { @@ -102,4 +103,42 @@ describe('SqlDriver.introspectSchema emits the spec introspection contract', () expect.arrayContaining(['name', 'type', 'nullable', 'primaryKey']), ); }); + + it('OMITS `indexes` — never a false-empty `[]` (#11122)', async () => { + const schema = await driver.introspectSchema(); + + // This driver does not read indexes, and the spec key is optional with + // absence meaning exactly that. `indexes: []` would instead be a positive + // claim the table HAS none — the answer a drift differ acts on — so the + // pin is on key ABSENCE, `in`, not on emptiness. + expect('indexes' in schema.tables['customers']).toBe(false); + + // And the emitted value now satisfies the spec declaration AS DECLARED — + // this assignment is the compile-time half of the pin, the seam that had + // no compiler across it while the `Omit` workarounds stood. Restoring the + // required `indexes` (or the `string` defaultValue) in the spec turns + // this file's `tsc` red at the driver's construction site. + const asSpec: SpecIntrospectedSchema = schema; + expect(asSpec.tables['customers'].indexes).toBeUndefined(); + }); + + it('emits `defaultValue` / `maxLength` raw — the measured shapes the widened declarations promise (#11122)', async () => { + const schema = await driver.introspectSchema(); + const byName = Object.fromEntries( + schema.tables['customers'].columns.map((c) => [c.name, c]), + ); + + // Measured on live in-memory SQLite (knex `columnInfo()` pass-through): + // a column with no default reports `null` — the value the old + // `defaultValue?: string` declaration could not even spell. + expect(byName.id.defaultValue).toBeNull(); + + // varchar maxLength arrives as the STRING "255" on SQLite while other + // dialects report a number — the measurement behind `number | string`. + // Asserted through Number() so a knex release normalising the spelling + // does not read as a contract break; typeof stays inside the declared + // union either way. + expect(['number', 'string']).toContain(typeof byName.name.maxLength); + expect(Number(byName.name.maxLength)).toBe(255); + }); }); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index ecc75e8432..d98efdd606 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -3673,24 +3673,25 @@ function nullSafeNegationOperand(node: Record): Record { - /** Raw driver-reported default. See the note above on why this is not `string`. */ - defaultValue?: unknown; +export interface IntrospectedColumn extends SpecIntrospectedColumn { /** SQL-introspection extra: the column carries a UNIQUE constraint. */ isUnique?: boolean; - /** SQL-introspection extra: declared maximum length for string types. */ - maxLength?: number; + /** + * SQL-introspection extra: declared maximum length for string types — raw + * as knex `columnInfo()` reports it, which is a NUMBER on some dialects + * and a STRING on SQLite (measured live, 2026-08-23: `"255"` for a + * `t.string(…)` column). Consumers narrow via `typeof`, as + * `schema-drift.ts`'s varchar differ already does. + */ + maxLength?: number | string; } /** No spec counterpart — foreign keys are a SQL-introspection extra. */ @@ -3702,13 +3703,15 @@ export interface IntrospectedForeignKey { } /** - * `indexes` is `Omit`ted from the spec table rather than emitted empty: this - * driver does not introspect indexes, and `indexes: []` would tell a schema - * differ that a table HAS none when it merely was not asked — a worse answer - * than an absent key. Emitting them for real is per-dialect work over arms - * this container cannot execute, so it is filed rather than guessed. + * `indexes` is inherited from the spec contract, where it is OPTIONAL and + * absence is meaningful (#11122): this driver does not introspect indexes, + * so `introspectSchema` deliberately OMITS the key — `indexes: []` would + * tell a schema differ that a table HAS none when it merely was not asked, + * a worse answer than an absent key. Wiring the real read + * ({@link SqlDriver.introspectIndexes} exists) is explicitly a separate + * decision with its own `onFailure` ruling. */ -export interface IntrospectedTable extends Omit { +export interface IntrospectedTable extends SpecIntrospectedTable { columns: IntrospectedColumn[]; /** SQL-introspection extra: outbound foreign keys. */ foreignKeys: IntrospectedForeignKey[]; @@ -3722,7 +3725,7 @@ export interface IntrospectedTable extends Omit { +export interface IntrospectedSchema extends SpecIntrospectedSchema { tables: Record; } @@ -12925,7 +12928,9 @@ export class SqlDriver implements IDataDriver { for (const colName of orderedNames) { const info = columnInfo[colName]; let type = 'string'; - let maxLength: number | undefined; + // Raw as knex reports it: a number on some dialects, a STRING on SQLite + // (measured: `"255"`) — see the `maxLength` note on IntrospectedColumn. + let maxLength: number | string | undefined; if (this.isSqlite) { type = info.type?.toLowerCase() || 'string'; diff --git a/packages/objectql/src/util.ts b/packages/objectql/src/util.ts index 264da21e4e..0f2ea015ad 100644 --- a/packages/objectql/src/util.ts +++ b/packages/objectql/src/util.ts @@ -24,20 +24,20 @@ import type { * to it, and this local declaration converges on the spec import rather than * keeping a second contract. * - * `defaultValue` stays `unknown` rather than the spec's `string`: SQL - * introspection reports whatever the driver read (`null`, a number, a boolean - * literal), and narrowing the declaration without normalising the value would - * move the lie rather than remove it. `isUnique` / `maxLength` are extra facts - * SQL introspection carries and the spec's diff-facing contract does not - * declare. + * The `Omit` carve-out that used to keep `defaultValue` diverging from the + * spec's `string` is RETIRED (#11122): the spec itself now declares the raw + * `unknown` producers actually report, so the key is inherited. `isUnique` / + * `maxLength` are extra facts SQL introspection carries and the spec's + * diff-facing contract does not declare. */ -export interface IntrospectedColumn extends Omit { - /** Default value if any — raw, as the driver reported it. */ - defaultValue?: unknown; +export interface IntrospectedColumn extends SpecIntrospectedColumn { /** Whether this column has a unique constraint */ isUnique?: boolean; - /** Maximum length for string types */ - maxLength?: number; + /** + * Maximum length for string types — raw as knex `columnInfo()` reports it: + * a number on some dialects, a STRING on SQLite (measured: `"255"`). + */ + maxLength?: number | string; } /** @@ -58,12 +58,13 @@ export interface IntrospectedForeignKey { * Table metadata from database introspection. * * DERIVED from the spec contract, like {@link IntrospectedColumn}. `indexes` - * is `Omit`ted rather than required: SQL introspection here does not read - * indexes, and an empty array would claim a table HAS none when it merely was - * not asked. `foreignKeys` / `primaryKeys` are extras the spec's diff-facing - * contract does not declare. + * is inherited as the spec's OPTIONAL key (#11122): a producer that did not + * read indexes omits the key, and an empty array is a positive claim that a + * table HAS none — so nothing here emits `[]` for "not asked". + * `foreignKeys` / `primaryKeys` are extras the spec's diff-facing contract + * does not declare. */ -export interface IntrospectedTable extends Omit { +export interface IntrospectedTable extends SpecIntrospectedTable { /** List of columns */ columns: IntrospectedColumn[]; /** List of foreign key relationships */ @@ -80,7 +81,7 @@ export interface IntrospectedTable extends Omit { +export interface IntrospectedSchema extends SpecIntrospectedSchema { /** Map of table name to table metadata */ tables: Record; } diff --git a/packages/spec/src/contracts/schema-diff-service.test.ts b/packages/spec/src/contracts/schema-diff-service.test.ts new file mode 100644 index 0000000000..2940842160 --- /dev/null +++ b/packages/spec/src/contracts/schema-diff-service.test.ts @@ -0,0 +1,91 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Pins for the two introspection-contract keys ruled in #11122 (maintainer + * ruling 2026-08-23, option B — 「其他同意你的意见」): `IntrospectedTable.indexes` + * is OPTIONAL (the required declaration was a promise no producer ever + * honoured), and `IntrospectedColumn.defaultValue` is the raw `unknown` + * producers actually report, not the `string` it used to promise. + * + * The load-bearing assertions here are COMPILE-TIME: the typed literals below + * are exactly what the old declarations refused or lied about. Restoring + * `indexes: IntrospectedIndex[]` (required) makes the no-indexes table literal + * fail `tsc`; restoring `defaultValue?: string` makes the `null` / `true` + * literals fail. The runtime expects only keep vitest honest about the same + * values. + */ + +import { describe, it, expect } from 'vitest'; +import type { + IntrospectedColumn, + IntrospectedSchema, + IntrospectedTable, +} from './schema-diff-service'; + +const col = (name: string, extra: Partial = {}): IntrospectedColumn => ({ + name, + type: 'varchar', + nullable: true, + primaryKey: false, + ...extra, +}); + +describe('IntrospectedTable.indexes (#11122: optional, absence ≠ empty)', () => { + it('a table WITHOUT `indexes` typechecks — absence means "not read", and the in-tree producer emits exactly this', () => { + const table: IntrospectedTable = { + name: 'customers', + columns: [col('id', { primaryKey: true })], + // no `indexes` key: the producer did not read indexes. Under the old + // required declaration this literal did not compile, yet it is the only + // shape any producer ever emitted. + }; + expect('indexes' in table).toBe(false); + }); + + it('an empty array stays a DISTINCT legal claim: the table was read and HAS no indexes', () => { + const none: IntrospectedTable = { name: 't_none', columns: [col('a')], indexes: [] }; + const some: IntrospectedTable = { + name: 't_some', + columns: [col('a')], + indexes: [{ name: 'idx_a', columns: ['a'], unique: false }], + }; + expect(none.indexes).toEqual([]); + expect(some.indexes).toHaveLength(1); + }); + + it('a whole schema built from index-less tables satisfies the contract', () => { + const schema: IntrospectedSchema = { + dialect: 'sqlite', + introspectedAt: new Date(0).toISOString(), + tables: { customers: { name: 'customers', columns: [col('id')] } }, + }; + expect(Object.keys(schema.tables)).toEqual(['customers']); + }); +}); + +describe('IntrospectedColumn.defaultValue (#11122: raw `unknown`, not `string`)', () => { + it('accepts the measured emitted values — `null`, a dialect-quoted string, a native boolean', () => { + // Measured on live in-memory SQLite (knex `columnInfo()` pass-through, + // 2026-08-23): `null` for a column with no default; dialect-quoted + // strings such as `'abc'` when one exists. The boolean mirrors the + // in-tree fixture for producers that report native values. + const noDefault: IntrospectedColumn = col('id', { defaultValue: null }); + const quoted: IntrospectedColumn = col('name', { defaultValue: "'abc'" }); + const native: IntrospectedColumn = col('active', { defaultValue: true }); + const absent: IntrospectedColumn = col('note'); + + expect(noDefault.defaultValue).toBeNull(); + expect(quoted.defaultValue).toBe("'abc'"); + expect(native.defaultValue).toBe(true); + expect('defaultValue' in absent).toBe(false); + }); + + it('is `unknown`, so a consumer MUST narrow before string operations — the old declaration invited exactly that crash', () => { + const c = col('x', { defaultValue: null }); + // Compile-time half: `c.defaultValue.startsWith('a')` must NOT typecheck + // (that is the consumer bug the `string` promise produced at runtime). + // Spelled as a narrowing guard, the only legal read: + const asString = typeof c.defaultValue === 'string' ? c.defaultValue : undefined; + expect(asString).toBeUndefined(); + }); +}); diff --git a/packages/spec/src/contracts/schema-diff-service.ts b/packages/spec/src/contracts/schema-diff-service.ts index 6a14c10a9a..0016875fe9 100644 --- a/packages/spec/src/contracts/schema-diff-service.ts +++ b/packages/spec/src/contracts/schema-diff-service.ts @@ -39,8 +39,26 @@ export interface IntrospectedTable { name: string; /** Column definitions */ columns: IntrospectedColumn[]; - /** Index definitions */ - indexes: IntrospectedIndex[]; + /** + * Index definitions — OPTIONAL, and absence is meaningful. + * + * An ABSENT key means the producer did not read indexes (the in-tree SQL + * driver does not: wiring the read adds one index query per table to every + * `introspectSchema()` call — the whole-warehouse federation path included — + * and needs its own `onFailure` failure-policy ruling; see + * `SqlDriver.introspectIndexes`). An EMPTY ARRAY is a positive claim that + * the table HAS no indexes. A producer that did not look must OMIT the key + * rather than emit `[]`, so a schema differ can tell "not asked" from + * "none exist". + * + * History (#11122): declared required from the start yet emitted by no + * producer ever, so a consumer typed against the promise read `undefined` + * with no compiler complaint. Withdrawn to optional per maintainer ruling + * 2026-08-23 (option B — 「其他同意你的意见」); wiring the index read is + * explicitly NOT this change, and becomes a new card if real consumer + * demand appears. + */ + indexes?: IntrospectedIndex[]; } /** @@ -53,8 +71,21 @@ export interface IntrospectedColumn { type: string; /** Whether the column is nullable */ nullable: boolean; - /** Default value expression */ - defaultValue?: string; + /** + * The column's default, RAW as the driver reported it — `unknown`, not the + * `string` this key used to promise (#11122). + * + * The in-tree SQL driver passes `knex.columnInfo().defaultValue` through + * unchanged. Measured on live in-memory SQLite (2026-08-23): `null` for a + * column with no default; a dialect-decorated STRING when one exists + * (SQLite quotes — `'abc'`, and spells a boolean default `'1'`; Postgres + * additionally appends a `::type` cast). Other producers report native + * values (an in-tree fixture carries `true` for a boolean column), and + * per-dialect CONTENT is deliberately not claimed here. Consumers must + * narrow before use — or compare through a helper such as driver-sql's + * `physicalDefaultIsToken` — never assume a string. + */ + defaultValue?: unknown; /** Whether this column is a primary key */ primaryKey: boolean; }