diff --git a/.changeset/sql-driver-own-query-doors.md b/.changeset/sql-driver-own-query-doors.md new file mode 100644 index 0000000000..fd2990e41f --- /dev/null +++ b/.changeset/sql-driver-own-query-doors.md @@ -0,0 +1,34 @@ +--- +"@objectstack/driver-sql": major +"@objectstack/verify": major +--- + +refactor(driver-sql)!: `analyzeQuery` / `findWithWindowFunctions` 不再吃 `any`,窗口门自带扁平形类型 (#6212 批 A+E) + +#5181(PR #6076)收窄了 `IDataDriver` 声明的六个方法,#6075(PR #6210)让五个驱动的实现跟上。收尾漏下的是**驱动自有、不在 `IDataDriver` 上**的那批查询门:它们同样吃 query AST,签名却是 `any`。本次处理 SQL 驱动的两个。 + +`any` 在 query 参数上不是「对象名没检查」,而是**检查全关**:`where` 的 filter 方言、`orderBy` 的 sort node 形状、`limit`/`offset` 是不是数字,全部被抹掉——而这两个方法体读的恰恰就是这些字段。`$like` 当年就是从同一个口子活到运行时的(cloud#1030、cloud#1053 实测 20 处)。 + +**`analyzeQuery` → `DriverQuery`。** 它是 `explain()` 的实现体,而 `explain()` 本来就声明 `DriverQuery` 并一行转发过来——收窄前这一对是自相矛盾的:契约门声明 AST,它背后的实现声明 `any`。方法体只读 `fields` / `where` / `orderBy` / `limit` / `offset`,全在 `DriverQuery` 内,因此这是一次纯注解:driver-sql 与 driver-sqlite-wasm 实测零报错、零 fixture 改动。 + +**`findWithWindowFunctions` → 驱动本地的扁平形类型**,新导出 `SqlWindowFunctionQuery` / `SqlWindowFunctionSpec`: + +```ts +import type { SqlWindowFunctionQuery } from '@objectstack/driver-sql'; + +const ranked = await sqlDriver.findWithWindowFunctions('employee', { + windowFunctions: [ + { function: 'rank', alias: 'salary_rank', partitionBy: ['department'], orderBy: [{ field: 'salary', order: 'desc' }] }, + ], +}); +``` + +它**不能**标 `DriverQuery`:`query.windowFunctions` 在 spec 是 `retiredKey()` 墓碑(#4286),`QueryAST['windowFunctions']` 解析为 `undefined`,标上去会让这道门自己已发布文档里的载荷编译不过。类型因此写成 `Omit & { windowFunctions?: SqlWindowFunctionSpec[] }`——契约那一半照旧受检,驱动私有那一半由驱动自己声明。 + +类型放在驱动层、**不进 `packages/spec`**,是接着 #4286 的判断往下走:那次删掉 `WindowFunctionNodeSchema` 的理由正是它声明了 `field` / `over` / `frame` 这些门从不读的成员;再往 spec 加一套窗口词汇就是反悔那个判断。spec 的删除注记与 `migrations/registry.ts` 的迁移处方里逐字写着的 `{ function, alias, partitionBy?, orderBy? }`,就是这个类型的出处,三处必须始终说同一句话。请求面的墓碑**没有**被重新打开:`analyzeQuery('o', { windowFunctions: [...] })` 依然是编译错误。 + +**顺带(#6212 批 F)**:`@objectstack/verify` 的 `BucketableDriver.aggregate` 从 `query: unknown` 收到 `DriverQuery`。这是一个**已发布**的结构替身,cloud 的 driver-turso 照着它实现——声明 `unknown` 不叫「最小」,叫没检查,并且放任该文件里两处 AST 字面量各自把对象名多写一遍(#5181 的那种冗余)。同时删掉一处 `as never`:那个 cast 只是因为字面量推断把 `'count'` 放宽成了 `string`,注上类型就不需要它了。这里**不预断**驱动自身 `aggregate` 参数类型的收窄(#6212 批 B,排在 #6203 之后)——方法参数按双变比较,驱动那边声明 `any`、`QueryAST` 还是收窄后的类型,都照样满足这个替身。 + +**零运行时改动**,全部是类型注解与两处冗余键的删除(实测全仓驱动无一读 `query.object`)。测试:driver-sql 935、driver-sqlite-wasm 254、driver-turso 804、verify 17、dogfood 520 全绿。 + +**迁移面**:直接调用这两道门的嵌入方,把内联字面量里编译器指出来的键改对即可(TS2353)。本仓实测非测试生产者为零,两道门只有各自驱动包的测试在用,零处需要改动。标 major 的依据与 #5181 / #6075 一致:**源码级破坏性**(调用点内联字面量与 `BucketableDriver` 的导出形状),运行时行为零变化;`check:api-surface` 只记录导出的存在与否、不记录签名,所以这条说明是该变更唯一的下游载体。 diff --git a/packages/drivers/driver-sql/src/index.ts b/packages/drivers/driver-sql/src/index.ts index 80e662d8ad..5c83909899 100644 --- a/packages/drivers/driver-sql/src/index.ts +++ b/packages/drivers/driver-sql/src/index.ts @@ -10,6 +10,13 @@ export type { IntrospectedTable, IntrospectedColumn, IntrospectedForeignKey, + // The window-function door's driver-private input shape (#6212). Exported so + // an embedder calling `findWithWindowFunctions` — the migration prescription + // #4286 published for the retired `query.windowFunctions` — can name the type + // it must build, instead of reaching for `as any` and losing `where`/`orderBy` + // checking with it. + SqlWindowFunctionSpec, + SqlWindowFunctionQuery, } from './sql-driver.js'; // Managed-schema drift / reconcile (#2186), incl. the index dimension (#3728) diff --git a/packages/drivers/driver-sql/src/sql-driver-query-signature.test.ts b/packages/drivers/driver-sql/src/sql-driver-query-signature.test.ts index 870787336f..b7c249aba1 100644 --- a/packages/drivers/driver-sql/src/sql-driver-query-signature.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-query-signature.test.ts @@ -34,6 +34,7 @@ import { describe, it, expect } from 'vitest'; import type { DriverQuery } from '@objectstack/spec/contracts'; import { SqlDriver } from './sql-driver.js'; +import type { SqlWindowFunctionQuery, SqlWindowFunctionSpec } from './sql-driver.js'; /** Resolves to `'dropped'` only while `T` has no `object` key; `never` otherwise. */ type DropsObject = 'object' extends keyof T ? never : 'dropped'; @@ -80,3 +81,172 @@ describe('SqlDriver query signatures follow the DriverQuery contract (#6075)', ( expect(accepted.limit).toBe(10); }); }); + +/** + * The two SQL-driver-OWN query doors — not on `IDataDriver`, so #5181/#6075 + * never reached them and both kept `query: any` (#6212). + * + * `any` on a query parameter is not "unchecked object name"; it is every check + * off. `where`'s filter dialect, `orderBy`'s sort-node shape, `limit`/`offset` + * being numbers — all of it was erased, on doors whose bodies read exactly + * those members. That is the same class of hole `$like` walked through + * (cloud#1030) before the contract methods were narrowed. + * + * `DropsObject` below is doing double duty on purpose: `keyof any` is + * `string | number | symbol`, so `'object' extends keyof any` is TRUE and + * `DropsObject` resolves to `never`. Widening either parameter back to + * `any` therefore fails these assignments just as loudly as re-adding `object` + * would — one pin, both regressions. + */ +describe('SqlDriver own query doors declare a query type (#6212)', () => { + describe('analyzeQuery — the DriverQuery half (batch A)', () => { + it('takes `DriverQuery`, the same type `explain()` forwards to it', () => { + // `explain(object, query: DriverQuery)` is a one-line forward to + // `analyzeQuery`, so the pair was self-inconsistent: the contract door + // declared the AST and the implementation behind it declared `any`. + const perMethod: [ + DropsObject[1]>, + DropsObject[1]>, + ] = ['dropped', 'dropped']; + expect(perMethod).toHaveLength(2); + + // Assignability in the direction that matters: what `explain` hands over + // must be exactly what `analyzeQuery` accepts. + const forwarded: Parameters[1] = {} as Parameters[1]; + expect(forwarded).toBeDefined(); + }); + + it('accepts every member the body actually reads', () => { + // fields / where / orderBy / limit / offset — the whole read set, all + // inside `DriverQuery`. This is the measurement that made batch A a pure + // annotation: no fixture in driver-sql or driver-sqlite-wasm had to move. + const q: Parameters[1] = { + fields: ['id', 'amount'], + where: { $and: [{ status: 'completed' }, { amount: { $gt: 100 } }] }, + orderBy: [{ field: 'amount', order: 'desc' }], + limit: 10, + offset: 5, + }; + expect(q.limit).toBe(10); + }); + + it('rejects a key outside `DriverQuery` at the call site', () => { + // @ts-expect-error - 'sortBy' does not exist in type 'DriverQuery' (it is `orderBy`) + const misspelled: Parameters[1] = { sortBy: [{ field: 'amount' }] }; + expect(misspelled).toBeTruthy(); + }); + + it('does NOT reopen the retired request-surface keys', () => { + // `windowFunctions` is a `retiredKey()` tombstone on `QueryAST` (#4286). + // `analyzeQuery` never read it, and taking `DriverQuery` keeps it shut — + // the window door is a SEPARATE method with its own type, below. + const withWindows: Parameters[1] = { + // @ts-expect-error - `windowFunctions` was removed from the query surface (#4286) + windowFunctions: [{ function: 'rank', alias: 'r' }], + }; + expect(withWindows).toBeTruthy(); + }); + }); + + describe('findWithWindowFunctions — the local flat shape (batch E)', () => { + it('compiles the payload this door\'s own published documentation shows', () => { + // THE acceptance criterion. #4286 tombstoned `query.windowFunctions` and + // published this door as the migration prescription — in the tombstone + // message, `migrations/registry.ts`, five docs pages, the release notes + // and the upgrade guide. A type that rejects the payload those texts + // print would make the prescription uncompilable, which is why the door + // could not simply take `DriverQuery` (whose `windowFunctions` is the + // tombstone, i.e. `undefined`). + // + // Copied verbatim from content/docs/data-modeling/queries.mdx. + const documented: SqlWindowFunctionQuery = { + windowFunctions: [ + { + function: 'rank', + alias: 'salary_rank', + partitionBy: ['department'], + orderBy: [{ field: 'salary', order: 'desc' }], + }, + ], + }; + const accepted: Parameters[1] = documented; + expect(accepted.windowFunctions?.[0]?.alias).toBe('salary_rank'); + }); + + it('keeps the contract half of the query checked', () => { + // The point of `Omit & …` rather than a + // bare `{ windowFunctions?: … }`: `where` / `orderBy` / `limit` / `offset` + // are read by this body too and are now checked exactly as on `find()`. + const q: Parameters[1] = { + where: { status: 'completed' }, + orderBy: [{ field: 'amount', order: 'desc' }], + limit: 5, + offset: 1, + windowFunctions: [{ function: 'ROW_NUMBER', alias: 'row_num' }], + }; + expect(q.limit).toBe(5); + }); + + it('drops the redundant object name like every other query door', () => { + const dropped: DropsObject[1]> = 'dropped'; + expect(dropped).toBe('dropped'); + // @ts-expect-error - 'object' does not exist in type 'SqlWindowFunctionQuery' + const redundant: SqlWindowFunctionQuery = { object: 'employee', windowFunctions: [] }; + expect(redundant).toBeTruthy(); + }); + + it('rejects the SPEC vocabulary #4286 removed — the shapes the builder never read', () => { + // `WindowFunctionNodeSchema` declared `field` / `over` / `frame`; + // `buildWindowFunction` reads none of them (it emits `FUNC()` with no + // argument at all, so `lag(revenue)` renders `LAG()`). #4286 deleted that + // cluster rather than leave a false affordance, and re-declaring it here + // would be that same false affordance one layer down. + const withRemovedMembers: SqlWindowFunctionSpec[] = [ + // @ts-expect-error - `field` / `over` / `frame` are the removed spec vocabulary; this door has none of them + { function: 'lag', alias: 'prev', field: 'revenue', over: { partitionBy: ['dept'] }, frame: 'rows' }, + ]; + expect(withRemovedMembers).toHaveLength(1); + }); + + it('requires the two members the builder cannot run without', () => { + // `spec.function.toUpperCase()` and the `as ??` binding on `wf.alias` + // both dereference unconditionally — absence is a runtime failure, so it + // is a compile failure. + // @ts-expect-error - Property 'alias' is missing + const noAlias: SqlWindowFunctionSpec = { function: 'rank' }; + // @ts-expect-error - Property 'function' is missing + const noFunction: SqlWindowFunctionSpec = { alias: 'r' }; + expect([noAlias, noFunction]).toHaveLength(2); + }); + + it('accepts an inner sort without `order` — the builder defaults it', () => { + // A `DriverQuery`'s top-level `SortNode` has `order` REQUIRED (the Zod + // default is applied in the output type). Inside `OVER (…)` the builder + // reads `s.order || 'asc'`, so absence is a spelling this door genuinely + // accepts and the local type must not over-declare. + const spec: SqlWindowFunctionSpec = { + function: 'ROW_NUMBER', + alias: 'n', + orderBy: [{ field: 'amount' }], + }; + expect(spec.orderBy?.[0]?.order).toBeUndefined(); + }); + + it('pins WHY the type `Omit`s the tombstoned key instead of intersecting it', () => { + // `query.windowFunctions` is `retiredKey(...)` — `z.never().optional()` — + // so `DriverQuery['windowFunctions']` is `undefined`. A plain + // `DriverQuery & { windowFunctions?: SqlWindowFunctionSpec[] }` therefore + // intersects an array with `undefined` and leaves the property + // UNWRITABLE: the door's own documented payload would stop compiling, + // silently, with no error anywhere near the type declaration. + // + // This assertion is that trap, held still. Simplify the `Omit` away and + // `naive` goes red here rather than in the docs. + type Writable = SqlWindowFunctionSpec[] extends NonNullable ? 'writable' : 'unwritable'; + const naive: Writable<(DriverQuery & { windowFunctions?: SqlWindowFunctionSpec[] })['windowFunctions']> = + 'unwritable'; + const real: Writable = 'writable'; + expect([naive, real]).toEqual(['unwritable', 'writable']); + }); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index d7870a4ea7..21e6043636 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -1688,6 +1688,63 @@ export interface IntrospectedSchema { tables: Record; } +// ── Window Function Types (driver-private, #6212) ──────────────────────────── + +/** + * One entry of {@link SqlWindowFunctionQuery.windowFunctions} — the flat shape + * {@link SqlDriver.findWithWindowFunctions} actually reads. + * + * This type lives HERE, not in `packages/spec`, deliberately. #4286 retired the + * spec's window cluster (`WindowFunctionNodeSchema` and friends) precisely + * because it declared `field` / `over` / `frame` members this door never read — + * a vocabulary describing an input no executor accepts. Re-adding a window + * vocabulary to the spec would undo that judgement; window functions are a + * SQL-driver-private capability (the door is not on `IDataDriver`), so the + * driver declares its own shape at the layer that owns it. The spec's own + * removal note names this shape verbatim — `{ function, alias, partitionBy?, + * orderBy? }`, `packages/spec/src/data/query.zod.ts` — as does the published + * migration prescription (`query-window-functions-retired` in + * `packages/spec/src/migrations/registry.ts`), which points embedders at this + * door. Those two texts and this type must keep saying the same thing. + * + * Every member is what {@link SqlDriver.buildWindowFunction} consumes and + * nothing else: + * - `function` is emitted as `FUNC()` — uppercased, ARGUMENT-LESS. `lag(revenue)` + * renders as `LAG()`; the builder has no argument slot, which is why there is + * no `field` member to declare (the skills' aggregation rules say the same). + * - `orderBy`'s `order` is optional here even though a `DriverQuery`'s top-level + * `SortNode` requires it: the builder reads `s.order || 'asc'`, so absence is + * a spelling this door genuinely accepts. Declaring it required would reject + * input that works. + */ +export interface SqlWindowFunctionSpec { + /** Window function name, emitted argument-less and uppercased (`rank` → `RANK()`). */ + function: string; + /** Column alias the computed value is projected as. */ + alias: string; + /** `PARTITION BY` targets, mapped through the driver's storage-name mapping. */ + partitionBy?: string[]; + /** `ORDER BY` inside the `OVER (…)` clause; `order` defaults to `asc`. */ + orderBy?: { field: string; order?: 'asc' | 'desc' }[]; +} + +/** + * The query {@link SqlDriver.findWithWindowFunctions} takes: a + * {@link DriverQuery} — the contract shape, minus the redundant `object` the + * first argument already carries (#5181) — carrying this driver's private + * `windowFunctions` array. + * + * `windowFunctions` is `Omit`ed off `DriverQuery` before being re-declared + * because the spec key is a `retiredKey()` TOMBSTONE: `QueryAST['windowFunctions']` + * resolves to `undefined`, so a plain intersection would leave the property + * unwritable and this door's own documented payload would not compile. The + * tombstone is correct — the REQUEST surface really has no window functions — + * and this type is what keeps the driver-level door open without reopening it. + */ +export type SqlWindowFunctionQuery = Omit & { + windowFunctions?: SqlWindowFunctionSpec[]; +}; + // ── Configuration Types ────────────────────────────────────────────────────── /** @@ -3654,7 +3711,15 @@ export class SqlDriver implements IDataDriver { // Window Functions // =================================== - async findWithWindowFunctions(object: string, query: any, options?: DriverOptions): Promise { + /** + * The one live window-function door (#4286): not on `IDataDriver`, callable + * directly on a SQL driver instance. Takes {@link SqlWindowFunctionQuery} — + * the contract query shape plus this driver's private `windowFunctions` + * array — so `where` / `orderBy` / `limit` / `offset` are checked here + * exactly as they are on `find()`, instead of being erased along with the + * driver-private part (#6212). + */ + async findWithWindowFunctions(object: string, query: SqlWindowFunctionQuery, options?: DriverOptions): Promise { const builder = this.getBuilder(object, options); builder.select('*'); @@ -3691,7 +3756,13 @@ export class SqlDriver implements IDataDriver { return this.analyzeQuery(object, query, options); } - async analyzeQuery(object: string, query: any, options?: DriverOptions): Promise { + /** + * `explain()`'s implementation, and the only other caller of it. It reads + * `fields` / `where` / `orderBy` / `limit` / `offset` — every one of them a + * `DriverQuery` member — so it takes `DriverQuery`, which is what `explain()` + * already declared and forwarded here (#6212). + */ + async analyzeQuery(object: string, query: DriverQuery, options?: DriverOptions): Promise { const builder = this.getBuilder(object, options); if (query.fields) { @@ -7369,7 +7440,7 @@ export class SqlDriver implements IDataDriver { // ── Window function builder ───────────────────────────────────────────────── - protected buildWindowFunction(spec: any): string { + protected buildWindowFunction(spec: SqlWindowFunctionSpec): string { const func = spec.function.toUpperCase(); let sql = `${func}()`; @@ -7382,7 +7453,7 @@ export class SqlDriver implements IDataDriver { if (spec.orderBy && Array.isArray(spec.orderBy) && spec.orderBy.length > 0) { const orderFields = spec.orderBy - .map((s: any) => { + .map((s) => { const field = this.mapSortField(s.field); const order = (s.order || 'asc').toUpperCase(); return `${field} ${order}`; diff --git a/packages/verify/src/date-bucket-parity.ts b/packages/verify/src/date-bucket-parity.ts index 3da36fe231..167fb376f8 100644 --- a/packages/verify/src/date-bucket-parity.ts +++ b/packages/verify/src/date-bucket-parity.ts @@ -40,15 +40,33 @@ */ import { applyInMemoryAggregation } from '@objectstack/objectql'; +import type { DriverQuery } from '@objectstack/spec/contracts'; -/** The minimal driver surface this check drives. */ +/** + * The minimal driver surface this check drives. + * + * `aggregate` names {@link DriverQuery} rather than `unknown` (#6212). A + * structural double declaring `unknown` is not "minimal", it is unchecked: it + * tells an out-of-tree driver author — this file is a PUBLISHED reference, and + * cloud's `driver-turso` implements it — nothing about the AST the check will + * send, and it left the two literals below free to drift from the shape the + * real drivers parse. `DriverQuery` is also what says the query does NOT repeat + * the object name that argument one already carries (#5181): the redundancy + * those literals used to spell is now spelled once. + * + * This deliberately does NOT presume what the drivers' own `aggregate` + * parameter type becomes when it is narrowed in its own right (#6212 batch B, + * behind #6203): method parameters compare bivariantly, so a driver declaring + * `any`, `QueryAST` or a narrowed type all satisfy this double either way. + * `find` stays `unknown` — this check never calls it with a query. + */ export interface BucketableDriver { connect?(): Promise; disconnect?(): Promise; syncSchema(object: string, schema: unknown, options?: unknown): Promise; create(object: string, data: Record, options?: unknown): Promise; find(object: string, query: unknown, options?: unknown): Promise; - aggregate(object: string, query: unknown, options?: unknown): Promise; + aggregate(object: string, query: DriverQuery, options?: unknown): Promise; supports?: { queryDateGranularity?: Record }; } @@ -197,8 +215,12 @@ export async function checkDateBucketParity( for (const granularity of GRANULARITIES) { if (caps[granularity] !== true) continue; // engine routes this in-memory - const ast = { - object, + // Annotated, not inferred: the annotation is what makes `'count'` keep + // its literal type and be checked against the declared aggregate + // vocabulary. Inferred, it widened to `string` — which is precisely why + // handing it to `applyInMemoryAggregation` needed an `as never` below, + // an erasure that would have swallowed a genuinely wrong AST here too. + const ast: DriverQuery = { groupBy: [{ field, dateGranularity: granularity }], aggregations: [{ function: 'count', alias: 'n' }], }; @@ -214,7 +236,7 @@ export async function checkDateBucketParity( } const sql = labelCounts(pushedDown, field); - const inMemory = labelCounts(applyInMemoryAggregation(rows, ast as never), field); + const inMemory = labelCounts(applyInMemoryAggregation(rows, ast), field); if (canonical(sql) !== canonical(inMemory)) { problems.push( @@ -231,7 +253,6 @@ export async function checkDateBucketParity( if (caps[granularity] !== true) continue; const mk = (field: 'at' | 'on') => driver.aggregate(object, { - object, groupBy: [{ field, dateGranularity: granularity }], aggregations: [{ function: 'count', alias: 'n' }], });