Skip to content

Commit d48aad5

Browse files
os-zhuangclaude
andauthored
refactor(driver-sql)!: analyzeQuery / findWithWindowFunctions declare a query type (#6212 batch A+E) (#6355)
`IDataDriver`'s six query methods were narrowed to `DriverQuery` by #5181 and followed through on five drivers by #6075. The SQL driver's OWN query doors — not on the contract, therefore never in scope of either — kept `query: any`. `any` there is not "the object name goes unchecked", it is every check off: `where`'s filter dialect, `orderBy`'s sort-node shape, `limit`/`offset` being numbers. Those are exactly the members both bodies read. - `analyzeQuery` -> `DriverQuery`. It is `explain()`'s implementation and `explain()` already declared `DriverQuery` and forwarded here, so the pair was self-inconsistent. Pure annotation: zero errors and zero fixture changes in driver-sql and driver-sqlite-wasm. - `findWithWindowFunctions` -> `SqlWindowFunctionQuery`, a driver-local flat type (exported, with `SqlWindowFunctionSpec`). It cannot take `DriverQuery`: `query.windowFunctions` is a `retiredKey()` tombstone since #4286, so `QueryAST['windowFunctions']` is `undefined` and this door's own published payload would stop compiling. The type is `Omit`-ed, not intersected, for exactly that reason, and a pin holds that trap still. It stays OUT of `packages/spec` deliberately: #4286 removed `WindowFunctionNodeSchema` because it declared `field`/`over`/`frame` members this door never reads, and the door's flat shape is quoted verbatim by the spec's removal note and the published migration prescription. - `buildWindowFunction(spec: any)` follows to `SqlWindowFunctionSpec`. Also (#6212 batch F): `@objectstack/verify`'s `BucketableDriver.aggregate` declares `DriverQuery` instead of `unknown`. It is a PUBLISHED structural double an out-of-tree driver implements, and `unknown` let the file's own two aggregate literals each repeat the object name argument one carries. Drops one `as never` that only existed because an inferred literal widened `'count'` to `string`. It does not presume batch B's choice for the drivers' own `aggregate` parameter — method parameters compare bivariantly either way. Zero runtime change: type annotations plus two redundant keys removed (no driver reads `query.object`). Part of #6212 Claude-Session: https://claude.ai/code/session_01WyvqvKMG6asi9aXjKE6xtx Co-authored-by: Claude <noreply@anthropic.com>
1 parent 262e40d commit d48aad5

5 files changed

Lines changed: 313 additions & 10 deletions

File tree

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
"@objectstack/driver-sql": major
3+
"@objectstack/verify": major
4+
---
5+
6+
refactor(driver-sql)!: `analyzeQuery` / `findWithWindowFunctions` 不再吃 `any`,窗口门自带扁平形类型 (#6212 批 A+E)
7+
8+
#5181(PR #6076)收窄了 `IDataDriver` 声明的六个方法,#6075(PR #6210)让五个驱动的实现跟上。收尾漏下的是**驱动自有、不在 `IDataDriver`**的那批查询门:它们同样吃 query AST,签名却是 `any`。本次处理 SQL 驱动的两个。
9+
10+
`any` 在 query 参数上不是「对象名没检查」,而是**检查全关**`where` 的 filter 方言、`orderBy` 的 sort node 形状、`limit`/`offset` 是不是数字,全部被抹掉——而这两个方法体读的恰恰就是这些字段。`$like` 当年就是从同一个口子活到运行时的(cloud#1030、cloud#1053 实测 20 处)。
11+
12+
**`analyzeQuery``DriverQuery`** 它是 `explain()` 的实现体,而 `explain()` 本来就声明 `DriverQuery` 并一行转发过来——收窄前这一对是自相矛盾的:契约门声明 AST,它背后的实现声明 `any`。方法体只读 `fields` / `where` / `orderBy` / `limit` / `offset`,全在 `DriverQuery` 内,因此这是一次纯注解:driver-sql 与 driver-sqlite-wasm 实测零报错、零 fixture 改动。
13+
14+
**`findWithWindowFunctions` → 驱动本地的扁平形类型**,新导出 `SqlWindowFunctionQuery` / `SqlWindowFunctionSpec`
15+
16+
```ts
17+
import type { SqlWindowFunctionQuery } from '@objectstack/driver-sql';
18+
19+
const ranked = await sqlDriver.findWithWindowFunctions('employee', {
20+
windowFunctions: [
21+
{ function: 'rank', alias: 'salary_rank', partitionBy: ['department'], orderBy: [{ field: 'salary', order: 'desc' }] },
22+
],
23+
});
24+
```
25+
26+
**不能**`DriverQuery``query.windowFunctions` 在 spec 是 `retiredKey()` 墓碑(#4286),`QueryAST['windowFunctions']` 解析为 `undefined`,标上去会让这道门自己已发布文档里的载荷编译不过。类型因此写成 `Omit<DriverQuery, 'windowFunctions'> & { windowFunctions?: SqlWindowFunctionSpec[] }`——契约那一半照旧受检,驱动私有那一半由驱动自己声明。
27+
28+
类型放在驱动层、**不进 `packages/spec`**,是接着 #4286 的判断往下走:那次删掉 `WindowFunctionNodeSchema` 的理由正是它声明了 `field` / `over` / `frame` 这些门从不读的成员;再往 spec 加一套窗口词汇就是反悔那个判断。spec 的删除注记与 `migrations/registry.ts` 的迁移处方里逐字写着的 `{ function, alias, partitionBy?, orderBy? }`,就是这个类型的出处,三处必须始终说同一句话。请求面的墓碑**没有**被重新打开:`analyzeQuery('o', { windowFunctions: [...] })` 依然是编译错误。
29+
30+
**顺带(#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` 还是收窄后的类型,都照样满足这个替身。
31+
32+
**零运行时改动**,全部是类型注解与两处冗余键的删除(实测全仓驱动无一读 `query.object`)。测试:driver-sql 935、driver-sqlite-wasm 254、driver-turso 804、verify 17、dogfood 520 全绿。
33+
34+
**迁移面**:直接调用这两道门的嵌入方,把内联字面量里编译器指出来的键改对即可(TS2353)。本仓实测非测试生产者为零,两道门只有各自驱动包的测试在用,零处需要改动。标 major 的依据与 #5181 / #6075 一致:**源码级破坏性**(调用点内联字面量与 `BucketableDriver` 的导出形状),运行时行为零变化;`check:api-surface` 只记录导出的存在与否、不记录签名,所以这条说明是该变更唯一的下游载体。

packages/drivers/driver-sql/src/index.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,13 @@ export type {
1010
IntrospectedTable,
1111
IntrospectedColumn,
1212
IntrospectedForeignKey,
13+
// The window-function door's driver-private input shape (#6212). Exported so
14+
// an embedder calling `findWithWindowFunctions` — the migration prescription
15+
// #4286 published for the retired `query.windowFunctions` — can name the type
16+
// it must build, instead of reaching for `as any` and losing `where`/`orderBy`
17+
// checking with it.
18+
SqlWindowFunctionSpec,
19+
SqlWindowFunctionQuery,
1320
} from './sql-driver.js';
1421

1522
// Managed-schema drift / reconcile (#2186), incl. the index dimension (#3728)

packages/drivers/driver-sql/src/sql-driver-query-signature.test.ts

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import { describe, it, expect } from 'vitest';
3535
import type { DriverQuery } from '@objectstack/spec/contracts';
3636
import { SqlDriver } from './sql-driver.js';
37+
import type { SqlWindowFunctionQuery, SqlWindowFunctionSpec } from './sql-driver.js';
3738

3839
/** Resolves to `'dropped'` only while `T` has no `object` key; `never` otherwise. */
3940
type DropsObject<T> = 'object' extends keyof T ? never : 'dropped';
@@ -80,3 +81,172 @@ describe('SqlDriver query signatures follow the DriverQuery contract (#6075)', (
8081
expect(accepted.limit).toBe(10);
8182
});
8283
});
84+
85+
/**
86+
* The two SQL-driver-OWN query doors — not on `IDataDriver`, so #5181/#6075
87+
* never reached them and both kept `query: any` (#6212).
88+
*
89+
* `any` on a query parameter is not "unchecked object name"; it is every check
90+
* off. `where`'s filter dialect, `orderBy`'s sort-node shape, `limit`/`offset`
91+
* being numbers — all of it was erased, on doors whose bodies read exactly
92+
* those members. That is the same class of hole `$like` walked through
93+
* (cloud#1030) before the contract methods were narrowed.
94+
*
95+
* `DropsObject` below is doing double duty on purpose: `keyof any` is
96+
* `string | number | symbol`, so `'object' extends keyof any` is TRUE and
97+
* `DropsObject<any>` resolves to `never`. Widening either parameter back to
98+
* `any` therefore fails these assignments just as loudly as re-adding `object`
99+
* would — one pin, both regressions.
100+
*/
101+
describe('SqlDriver own query doors declare a query type (#6212)', () => {
102+
describe('analyzeQuery — the DriverQuery half (batch A)', () => {
103+
it('takes `DriverQuery`, the same type `explain()` forwards to it', () => {
104+
// `explain(object, query: DriverQuery)` is a one-line forward to
105+
// `analyzeQuery`, so the pair was self-inconsistent: the contract door
106+
// declared the AST and the implementation behind it declared `any`.
107+
const perMethod: [
108+
DropsObject<Parameters<SqlDriver['explain']>[1]>,
109+
DropsObject<Parameters<SqlDriver['analyzeQuery']>[1]>,
110+
] = ['dropped', 'dropped'];
111+
expect(perMethod).toHaveLength(2);
112+
113+
// Assignability in the direction that matters: what `explain` hands over
114+
// must be exactly what `analyzeQuery` accepts.
115+
const forwarded: Parameters<SqlDriver['analyzeQuery']>[1] = {} as Parameters<SqlDriver['explain']>[1];
116+
expect(forwarded).toBeDefined();
117+
});
118+
119+
it('accepts every member the body actually reads', () => {
120+
// fields / where / orderBy / limit / offset — the whole read set, all
121+
// inside `DriverQuery`. This is the measurement that made batch A a pure
122+
// annotation: no fixture in driver-sql or driver-sqlite-wasm had to move.
123+
const q: Parameters<SqlDriver['analyzeQuery']>[1] = {
124+
fields: ['id', 'amount'],
125+
where: { $and: [{ status: 'completed' }, { amount: { $gt: 100 } }] },
126+
orderBy: [{ field: 'amount', order: 'desc' }],
127+
limit: 10,
128+
offset: 5,
129+
};
130+
expect(q.limit).toBe(10);
131+
});
132+
133+
it('rejects a key outside `DriverQuery` at the call site', () => {
134+
// @ts-expect-error - 'sortBy' does not exist in type 'DriverQuery' (it is `orderBy`)
135+
const misspelled: Parameters<SqlDriver['analyzeQuery']>[1] = { sortBy: [{ field: 'amount' }] };
136+
expect(misspelled).toBeTruthy();
137+
});
138+
139+
it('does NOT reopen the retired request-surface keys', () => {
140+
// `windowFunctions` is a `retiredKey()` tombstone on `QueryAST` (#4286).
141+
// `analyzeQuery` never read it, and taking `DriverQuery` keeps it shut —
142+
// the window door is a SEPARATE method with its own type, below.
143+
const withWindows: Parameters<SqlDriver['analyzeQuery']>[1] = {
144+
// @ts-expect-error - `windowFunctions` was removed from the query surface (#4286)
145+
windowFunctions: [{ function: 'rank', alias: 'r' }],
146+
};
147+
expect(withWindows).toBeTruthy();
148+
});
149+
});
150+
151+
describe('findWithWindowFunctions — the local flat shape (batch E)', () => {
152+
it('compiles the payload this door\'s own published documentation shows', () => {
153+
// THE acceptance criterion. #4286 tombstoned `query.windowFunctions` and
154+
// published this door as the migration prescription — in the tombstone
155+
// message, `migrations/registry.ts`, five docs pages, the release notes
156+
// and the upgrade guide. A type that rejects the payload those texts
157+
// print would make the prescription uncompilable, which is why the door
158+
// could not simply take `DriverQuery` (whose `windowFunctions` is the
159+
// tombstone, i.e. `undefined`).
160+
//
161+
// Copied verbatim from content/docs/data-modeling/queries.mdx.
162+
const documented: SqlWindowFunctionQuery = {
163+
windowFunctions: [
164+
{
165+
function: 'rank',
166+
alias: 'salary_rank',
167+
partitionBy: ['department'],
168+
orderBy: [{ field: 'salary', order: 'desc' }],
169+
},
170+
],
171+
};
172+
const accepted: Parameters<SqlDriver['findWithWindowFunctions']>[1] = documented;
173+
expect(accepted.windowFunctions?.[0]?.alias).toBe('salary_rank');
174+
});
175+
176+
it('keeps the contract half of the query checked', () => {
177+
// The point of `Omit<DriverQuery, 'windowFunctions'> & …` rather than a
178+
// bare `{ windowFunctions?: … }`: `where` / `orderBy` / `limit` / `offset`
179+
// are read by this body too and are now checked exactly as on `find()`.
180+
const q: Parameters<SqlDriver['findWithWindowFunctions']>[1] = {
181+
where: { status: 'completed' },
182+
orderBy: [{ field: 'amount', order: 'desc' }],
183+
limit: 5,
184+
offset: 1,
185+
windowFunctions: [{ function: 'ROW_NUMBER', alias: 'row_num' }],
186+
};
187+
expect(q.limit).toBe(5);
188+
});
189+
190+
it('drops the redundant object name like every other query door', () => {
191+
const dropped: DropsObject<Parameters<SqlDriver['findWithWindowFunctions']>[1]> = 'dropped';
192+
expect(dropped).toBe('dropped');
193+
// @ts-expect-error - 'object' does not exist in type 'SqlWindowFunctionQuery'
194+
const redundant: SqlWindowFunctionQuery = { object: 'employee', windowFunctions: [] };
195+
expect(redundant).toBeTruthy();
196+
});
197+
198+
it('rejects the SPEC vocabulary #4286 removed — the shapes the builder never read', () => {
199+
// `WindowFunctionNodeSchema` declared `field` / `over` / `frame`;
200+
// `buildWindowFunction` reads none of them (it emits `FUNC()` with no
201+
// argument at all, so `lag(revenue)` renders `LAG()`). #4286 deleted that
202+
// cluster rather than leave a false affordance, and re-declaring it here
203+
// would be that same false affordance one layer down.
204+
const withRemovedMembers: SqlWindowFunctionSpec[] = [
205+
// @ts-expect-error - `field` / `over` / `frame` are the removed spec vocabulary; this door has none of them
206+
{ function: 'lag', alias: 'prev', field: 'revenue', over: { partitionBy: ['dept'] }, frame: 'rows' },
207+
];
208+
expect(withRemovedMembers).toHaveLength(1);
209+
});
210+
211+
it('requires the two members the builder cannot run without', () => {
212+
// `spec.function.toUpperCase()` and the `as ??` binding on `wf.alias`
213+
// both dereference unconditionally — absence is a runtime failure, so it
214+
// is a compile failure.
215+
// @ts-expect-error - Property 'alias' is missing
216+
const noAlias: SqlWindowFunctionSpec = { function: 'rank' };
217+
// @ts-expect-error - Property 'function' is missing
218+
const noFunction: SqlWindowFunctionSpec = { alias: 'r' };
219+
expect([noAlias, noFunction]).toHaveLength(2);
220+
});
221+
222+
it('accepts an inner sort without `order` — the builder defaults it', () => {
223+
// A `DriverQuery`'s top-level `SortNode` has `order` REQUIRED (the Zod
224+
// default is applied in the output type). Inside `OVER (…)` the builder
225+
// reads `s.order || 'asc'`, so absence is a spelling this door genuinely
226+
// accepts and the local type must not over-declare.
227+
const spec: SqlWindowFunctionSpec = {
228+
function: 'ROW_NUMBER',
229+
alias: 'n',
230+
orderBy: [{ field: 'amount' }],
231+
};
232+
expect(spec.orderBy?.[0]?.order).toBeUndefined();
233+
});
234+
235+
it('pins WHY the type `Omit`s the tombstoned key instead of intersecting it', () => {
236+
// `query.windowFunctions` is `retiredKey(...)` — `z.never().optional()` —
237+
// so `DriverQuery['windowFunctions']` is `undefined`. A plain
238+
// `DriverQuery & { windowFunctions?: SqlWindowFunctionSpec[] }` therefore
239+
// intersects an array with `undefined` and leaves the property
240+
// UNWRITABLE: the door's own documented payload would stop compiling,
241+
// silently, with no error anywhere near the type declaration.
242+
//
243+
// This assertion is that trap, held still. Simplify the `Omit` away and
244+
// `naive` goes red here rather than in the docs.
245+
type Writable<T> = SqlWindowFunctionSpec[] extends NonNullable<T> ? 'writable' : 'unwritable';
246+
const naive: Writable<(DriverQuery & { windowFunctions?: SqlWindowFunctionSpec[] })['windowFunctions']> =
247+
'unwritable';
248+
const real: Writable<SqlWindowFunctionQuery['windowFunctions']> = 'writable';
249+
expect([naive, real]).toEqual(['unwritable', 'writable']);
250+
});
251+
});
252+
});

packages/drivers/driver-sql/src/sql-driver.ts

Lines changed: 75 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1688,6 +1688,63 @@ export interface IntrospectedSchema {
16881688
tables: Record<string, IntrospectedTable>;
16891689
}
16901690

1691+
// ── Window Function Types (driver-private, #6212) ────────────────────────────
1692+
1693+
/**
1694+
* One entry of {@link SqlWindowFunctionQuery.windowFunctions} — the flat shape
1695+
* {@link SqlDriver.findWithWindowFunctions} actually reads.
1696+
*
1697+
* This type lives HERE, not in `packages/spec`, deliberately. #4286 retired the
1698+
* spec's window cluster (`WindowFunctionNodeSchema` and friends) precisely
1699+
* because it declared `field` / `over` / `frame` members this door never read —
1700+
* a vocabulary describing an input no executor accepts. Re-adding a window
1701+
* vocabulary to the spec would undo that judgement; window functions are a
1702+
* SQL-driver-private capability (the door is not on `IDataDriver`), so the
1703+
* driver declares its own shape at the layer that owns it. The spec's own
1704+
* removal note names this shape verbatim — `{ function, alias, partitionBy?,
1705+
* orderBy? }`, `packages/spec/src/data/query.zod.ts` — as does the published
1706+
* migration prescription (`query-window-functions-retired` in
1707+
* `packages/spec/src/migrations/registry.ts`), which points embedders at this
1708+
* door. Those two texts and this type must keep saying the same thing.
1709+
*
1710+
* Every member is what {@link SqlDriver.buildWindowFunction} consumes and
1711+
* nothing else:
1712+
* - `function` is emitted as `FUNC()` — uppercased, ARGUMENT-LESS. `lag(revenue)`
1713+
* renders as `LAG()`; the builder has no argument slot, which is why there is
1714+
* no `field` member to declare (the skills' aggregation rules say the same).
1715+
* - `orderBy`'s `order` is optional here even though a `DriverQuery`'s top-level
1716+
* `SortNode` requires it: the builder reads `s.order || 'asc'`, so absence is
1717+
* a spelling this door genuinely accepts. Declaring it required would reject
1718+
* input that works.
1719+
*/
1720+
export interface SqlWindowFunctionSpec {
1721+
/** Window function name, emitted argument-less and uppercased (`rank` → `RANK()`). */
1722+
function: string;
1723+
/** Column alias the computed value is projected as. */
1724+
alias: string;
1725+
/** `PARTITION BY` targets, mapped through the driver's storage-name mapping. */
1726+
partitionBy?: string[];
1727+
/** `ORDER BY` inside the `OVER (…)` clause; `order` defaults to `asc`. */
1728+
orderBy?: { field: string; order?: 'asc' | 'desc' }[];
1729+
}
1730+
1731+
/**
1732+
* The query {@link SqlDriver.findWithWindowFunctions} takes: a
1733+
* {@link DriverQuery} — the contract shape, minus the redundant `object` the
1734+
* first argument already carries (#5181) — carrying this driver's private
1735+
* `windowFunctions` array.
1736+
*
1737+
* `windowFunctions` is `Omit`ed off `DriverQuery` before being re-declared
1738+
* because the spec key is a `retiredKey()` TOMBSTONE: `QueryAST['windowFunctions']`
1739+
* resolves to `undefined`, so a plain intersection would leave the property
1740+
* unwritable and this door's own documented payload would not compile. The
1741+
* tombstone is correct — the REQUEST surface really has no window functions —
1742+
* and this type is what keeps the driver-level door open without reopening it.
1743+
*/
1744+
export type SqlWindowFunctionQuery = Omit<DriverQuery, 'windowFunctions'> & {
1745+
windowFunctions?: SqlWindowFunctionSpec[];
1746+
};
1747+
16911748
// ── Configuration Types ──────────────────────────────────────────────────────
16921749

16931750
/**
@@ -3654,7 +3711,15 @@ export class SqlDriver implements IDataDriver {
36543711
// Window Functions
36553712
// ===================================
36563713

3657-
async findWithWindowFunctions(object: string, query: any, options?: DriverOptions): Promise<any[]> {
3714+
/**
3715+
* The one live window-function door (#4286): not on `IDataDriver`, callable
3716+
* directly on a SQL driver instance. Takes {@link SqlWindowFunctionQuery} —
3717+
* the contract query shape plus this driver's private `windowFunctions`
3718+
* array — so `where` / `orderBy` / `limit` / `offset` are checked here
3719+
* exactly as they are on `find()`, instead of being erased along with the
3720+
* driver-private part (#6212).
3721+
*/
3722+
async findWithWindowFunctions(object: string, query: SqlWindowFunctionQuery, options?: DriverOptions): Promise<any[]> {
36583723
const builder = this.getBuilder(object, options);
36593724

36603725
builder.select('*');
@@ -3691,7 +3756,13 @@ export class SqlDriver implements IDataDriver {
36913756
return this.analyzeQuery(object, query, options);
36923757
}
36933758

3694-
async analyzeQuery(object: string, query: any, options?: DriverOptions): Promise<any> {
3759+
/**
3760+
* `explain()`'s implementation, and the only other caller of it. It reads
3761+
* `fields` / `where` / `orderBy` / `limit` / `offset` — every one of them a
3762+
* `DriverQuery` member — so it takes `DriverQuery`, which is what `explain()`
3763+
* already declared and forwarded here (#6212).
3764+
*/
3765+
async analyzeQuery(object: string, query: DriverQuery, options?: DriverOptions): Promise<any> {
36953766
const builder = this.getBuilder(object, options);
36963767

36973768
if (query.fields) {
@@ -7369,7 +7440,7 @@ export class SqlDriver implements IDataDriver {
73697440

73707441
// ── Window function builder ─────────────────────────────────────────────────
73717442

7372-
protected buildWindowFunction(spec: any): string {
7443+
protected buildWindowFunction(spec: SqlWindowFunctionSpec): string {
73737444
const func = spec.function.toUpperCase();
73747445
let sql = `${func}()`;
73757446

@@ -7382,7 +7453,7 @@ export class SqlDriver implements IDataDriver {
73827453

73837454
if (spec.orderBy && Array.isArray(spec.orderBy) && spec.orderBy.length > 0) {
73847455
const orderFields = spec.orderBy
7385-
.map((s: any) => {
7456+
.map((s) => {
73867457
const field = this.mapSortField(s.field);
73877458
const order = (s.order || 'asc').toUpperCase();
73887459
return `${field} ${order}`;

0 commit comments

Comments
 (0)