Remove legacy query key fallbacks (filters/sort/skip/top) from driver-sql by Copilot · Pull Request #961 · objectstack-ai/framework

Copilot · 2026-03-23T04:21:00Z

SqlDriver bypassed QueryAST type safety via as any casts to support deprecated keys (filters, sort, skip, top). This blocks downstream protocol enforcement and weakens the IDataDriver contract.

Driver changes (`sql-driver.ts`)

find() — Remove fallbacks to (query as any).filters, .sort, .skip, .top; use only query.where, query.orderBy, query.offset, query.limit
updateMany() / deleteMany() — Remove (query as any).filters || query pattern; use only query.where
count() — Remove (query as any).filters fallback; use only query?.where
applyFilters() — Remove filters, sort, skip from the object-key skip-list

Before:

const filterCondition = query.where || (query as any).filters;
const sortArray = query.orderBy || (query as any).sort;
const offsetValue = query.offset ?? (query as any).skip;
const limitValue = query.limit ?? (query as any).top;

After:

if (query.where) this.applyFilters(builder, query.where);
if (query.orderBy && Array.isArray(query.orderBy)) { /* ... */ }
if (query.offset !== undefined) builder.offset(query.offset);
if (query.limit !== undefined) builder.limit(query.limit);

Test updates

Converted all existing tests from legacy keys to standard QueryAST format (where/orderBy/offset/limit)
Added 7 tests asserting legacy keys are silently ignored (no filter/sort/pagination applied)
78/78 tests pass

Original prompt

This section details on the original issue you should resolve

<issue_title>【清理遗留】driver-sql 依然冗余支持 legacy 查询 key（filters/sort/skip/top）- 需严格遵循 IDataDriver 协议</issue_title>
<issue_description>## 问题描述
packages/plugins/driver-sql/src/sql-driver.ts 目前依然大量存在 filters / sort / skip / top 兼容写法，例如：
const filterCondition = query.where || (query as any).filters;
const sortArray = query.orderBy || (query as any).sort;
const offsetValue = query.offset ?? (query as any).skip;
const limitValue = query.limit ?? (query as any).top;
这与 IDataDriver 和 QueryAST 协议要求冲突：应只认标准字段如 where / orderBy / offset / limit，不再支持旧 key。

这些兼容实例多见于：

find

updateMany

deleteMany

count

影响

类型安全失效：as any 绕开类型断言，实际允许错用输入被悄然吞掉

协议弱化：新插件和 IDE/DevTool 无法强制严格按 @objectstack/spec 结构开发

架构割裂：声明规范、实现场景两个生态，无法推动整体切换

修正建议

全部移除 filters / sort / skip / top fallback：直接报错或忽略旧 key。

类型 check：as any 的地方返回类型错误直接 fail。

完善测试：确保只填错 key 就报错/无效。

验收标准

CI 跑过 type check

单元测试以纯 QueryAST 传参，所有老 key 全失败或无效

参考 Issue #958，当前此遗留清理为阻断下游协议推进的硬性要求。</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes 【清理遗留】driver-sql 依然冗余支持 legacy 查询 key（filters/sort/skip/top）- 需严格遵循 IDataDriver 协议 #960

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

vercel · 2026-03-23T04:21:05Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
objectstack-play	Ready	Preview, Comment	Mar 23, 2026 4:32am

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
spec	Ignored		Mar 23, 2026 4:32am

…river-sql Strictly enforce IDataDriver / QueryAST protocol in SqlDriver: - find(): remove fallback to (query as any).filters/.sort/.skip/.top - updateMany(): remove fallback to (query as any).filters - deleteMany(): remove fallback to (query as any).filters - count(): remove fallback to (query as any).filters - applyFilters(): clean up skip-list of legacy keys Update all tests to use standard QueryAST keys (where/orderBy/offset/limit). Add tests verifying legacy keys are ignored. Closes #960 Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> Agent-Logs-Url: https://github.com/objectstack-ai/spec/sessions/61332fa6-4cc7-491f-b72c-2c73c6357e02

Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> Agent-Logs-Url: https://github.com/objectstack-ai/spec/sessions/61332fa6-4cc7-491f-b72c-2c73c6357e02

Copilot

Pull request overview

This PR removes driver-sql’s legacy query-key fallbacks that previously bypassed QueryAST typing via as any, aiming to enforce stricter adherence to the IDataDriver / QueryAST protocol.

Changes:

Removed legacy fallbacks for filters/sort/skip/top in find(), plus legacy filter handling in updateMany(), deleteMany(), and count().
Updated tests to use standard QueryAST keys (where/orderBy/offset/limit) and added tests asserting legacy keys are ignored.
Documented the removal in CHANGELOG.md.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
packages/plugins/driver-sql/src/sql-driver.ts	Removes legacy key fallbacks in query handling and adjusts `applyFilters` skip-list.
packages/plugins/driver-sql/src/sql-driver.test.ts	Updates count test input to use `where`.
packages/plugins/driver-sql/src/sql-driver-queryast.test.ts	Migrates tests to standard keys and adds “legacy keys ignored” assertions.
packages/plugins/driver-sql/src/sql-driver-advanced.test.ts	Migrates updateMany/deleteMany/count tests to standard `where` usage.
CHANGELOG.md	Adds an “Unreleased → Removed” entry describing legacy-key fallback removal.

Copilot · 2026-03-23T04:46:31Z

+    it('should ignore legacy "top" key — only "limit" is recognized', async () => {
+      const results = await driver.find('products', {
+        top: 2,
+        orderBy: [{ field: 'name', order: 'asc' as const }],
+      } as any);
+
+      // "top" is not recognized — no limit applied, returns all 5 rows
+      expect(results.length).toBe(5);


This test asserts legacy top is ignored, but QueryAST in @objectstack/spec/data currently defines top as an alias for limit (OData compatibility). If the driver is meant to follow the spec, top should behave like limit and this test should be updated/removed. If top is truly being removed from the protocol, the spec/schema should be updated in the same change so the contract is consistent.

Suggested change

it('should ignore legacy "top" key — only "limit" is recognized', async () => {

const results = await driver.find('products', {

top: 2,

orderBy: [{ field: 'name', order: 'asc' as const }],

} as any);

// "top" is not recognized — no limit applied, returns all 5 rows

expect(results.length).toBe(5);

it('should treat legacy "top" key as alias for "limit"', async () => {

const results = await driver.find('products', {

top: 2,

orderBy: [{ field: 'name', order: 'asc' as const }],

} as any);

// "top" is defined in QueryAST as an alias for "limit" (OData compatibility) — applies a limit of 2

expect(results.length).toBe(2);

Copilot · 2026-03-23T04:46:32Z

+  query keys `filters` (use `where`), `sort` (use `orderBy`), `skip` (use `offset`), and `top`
+  (use `limit`) from `find`, `updateMany`, `deleteMany`, and `count` methods. The SQL driver now
+  strictly follows the `IDataDriver` / `QueryAST` protocol. All `as any` casts for legacy key
+  access have been eliminated. Tests updated to use only standard `QueryAST` keys.


The changelog entry claims top support was removed as a deprecated legacy key, but QueryAST in @objectstack/spec/data still defines top as an alias for limit. Either keep top supported in the driver (and adjust this entry), or update the spec/schema in the same release so the documented protocol and driver behavior don’t diverge.

Suggested change

query keys `filters` (use `where`), `sort` (use `orderBy`), `skip` (use `offset`), and `top`

(use `limit`) from `find`, `updateMany`, `deleteMany`, and `count` methods. The SQL driver now

strictly follows the `IDataDriver` / `QueryAST` protocol. All `as any` casts for legacy key

access have been eliminated. Tests updated to use only standard `QueryAST` keys.

query keys `filters` (use `where`), `sort` (use `orderBy`), and `skip` (use `offset`) from

`find`, `updateMany`, `deleteMany`, and `count` methods. The `top` key remains supported as an

alias for `limit` via the standard `QueryAST` contract in `@objectstack/spec/data`. The SQL

driver now strictly follows the `IDataDriver` / `QueryAST` protocol. All `as any` casts for

legacy key access have been eliminated. Tests updated to use only standard `QueryAST` keys and

no driver-specific legacy fallbacks.

Copilot · 2026-03-23T04:46:32Z

-    if (limitValue !== undefined) builder.limit(limitValue);
+    // PAGINATION
+    if (query.offset !== undefined) builder.offset(query.offset);
+    if (query.limit !== undefined) builder.limit(query.limit);


find() no longer recognizes the top alias for pagination. However, QueryAST in @objectstack/spec/data still includes top (described as an alias for limit for OData compatibility), so ignoring it breaks the QueryAST contract for SqlDriver consumers. Either map query.top → limit here, or remove top from the QueryAST schema/protocol first so the driver and spec stay consistent.

Suggested change

if (query.limit !== undefined) builder.limit(query.limit);

const limit = query.limit ?? query.top;

if (limit !== undefined) builder.limit(limit);

Copilot · 2026-03-23T04:46:32Z

  async updateMany(object: string, query: QueryAST, data: any, options?: DriverOptions): Promise<number> {
    const builder = this.getBuilder(object, options);
-    const filters = query.where || (query as any).filters || query;
-    if (filters) this.applyFilters(builder, filters);
+    if (query.where) this.applyFilters(builder, query.where);
    const count = await builder.update(data);
    return count || 0;
  }

  async deleteMany(object: string, query: QueryAST, options?: DriverOptions): Promise<number> {
    const builder = this.getBuilder(object, options);
-    const filters = query.where || (query as any).filters || query;
-    if (filters) this.applyFilters(builder, filters);
+    if (query.where) this.applyFilters(builder, query.where);
    const count = await builder.delete();


updateMany() / deleteMany() now apply filters only when query.where is present. If a caller still passes a legacy shape (e.g. { filters: ... } or the older { status: 'pending' } shorthand), the operation will run with no WHERE clause and can update/delete the entire table. To avoid destructive surprises, consider rejecting queries that lack where (or at least throw when legacy keys like filters/sort/skip/top are present without where), and require an explicit empty where to intentionally target all rows.

Copilot · 2026-03-23T04:46:32Z

+      for (const item of query.orderBy) {
        const field = item.field || item[0];
        const dir = item.order || item[1] || 'asc';
        if (field) {


The ORDER BY loop still supports tuple/array forms (item[0] / item[1]) even though the QueryAST spec’s orderBy is defined as an array of { field, order } objects. If the goal is strict QueryAST enforcement, this should only accept the spec shape and drop the tuple fallback (and update tests accordingly), otherwise the driver continues to accept non-spec input and type safety remains weakened.

Suggested change

for (const item of query.orderBy) {

const field = item.field || item[0];

const dir = item.order || item[1] || 'asc';

if (field) {

for (const { field, order } of query.orderBy as Array<{ field?: string; order?: 'asc' | 'desc' }>) {

if (field) {

const dir = order ?? 'asc';

Initial plan

fad2a3f

Copilot AI assigned Copilot and hotlong Mar 23, 2026

Copilot started work on behalf of hotlong March 23, 2026 04:21 View session

vercel Bot deployed to Preview – objectstack-play March 23, 2026 04:23 View deployment

vercel Bot deployed to Preview – objectstack-play March 23, 2026 04:30 View deployment

test: add stronger legacy-key-ignored tests per code review feedback

6a72327

Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> Agent-Logs-Url: https://github.com/objectstack-ai/spec/sessions/61332fa6-4cc7-491f-b72c-2c73c6357e02

Copilot AI changed the title ~~[WIP] Remove legacy query key support in driver-sql~~ Remove legacy query key fallbacks (filters/sort/skip/top) from driver-sql Mar 23, 2026

Copilot AI requested a review from hotlong March 23, 2026 04:31

Copilot finished work on behalf of hotlong March 23, 2026 04:31

vercel Bot deployed to Preview – objectstack-play March 23, 2026 04:32 View deployment

hotlong marked this pull request as ready for review March 23, 2026 04:42

Copilot AI review requested due to automatic review settings March 23, 2026 04:42

hotlong merged commit e001d61 into main Mar 23, 2026
4 checks passed

Copilot started reviewing on behalf of hotlong March 23, 2026 04:42 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove legacy query key fallbacks (filters/sort/skip/top) from driver-sql#961

Remove legacy query key fallbacks (filters/sort/skip/top) from driver-sql#961
hotlong merged 3 commits into
mainfrom
copilot/cleanup-legacy-driver-sql-support

Copilot AI commented Mar 23, 2026 •

edited

Loading

Uh oh!

vercel Bot commented Mar 23, 2026 •

edited

Loading

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-  query keys `filters` (use `where`), `sort` (use `orderBy`), `skip` (use `offset`), and `top`
-  (use `limit`) from `find`, `updateMany`, `deleteMany`, and `count` methods. The SQL driver now
-  strictly follows the `IDataDriver` / `QueryAST` protocol. All `as any` casts for legacy key
-  access have been eliminated. Tests updated to use only standard `QueryAST` keys.
+  query keys `filters` (use `where`), `sort` (use `orderBy`), and `skip` (use `offset`) from
+  `find`, `updateMany`, `deleteMany`, and `count` methods. The `top` key remains supported as an
+  alias for `limit` via the standard `QueryAST` contract in `@objectstack/spec/data`. The SQL
+  driver now strictly follows the `IDataDriver` / `QueryAST` protocol. All `as any` casts for
+  legacy key access have been eliminated. Tests updated to use only standard `QueryAST` keys and
+  no driver-specific legacy fallbacks.

	if (query.limit !== undefined) builder.limit(query.limit);
	const limit = query.limit ?? query.top;
	if (limit !== undefined) builder.limit(limit);

Uh oh!

Conversation

Copilot AI commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Driver changes (sql-driver.ts)

Test updates

影响

修正建议

验收标准

Comments on the Issue (you are @copilot in this section)

Uh oh!

vercel Bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Mar 23, 2026 •

edited

Loading

Driver changes (`sql-driver.ts`)

vercel Bot commented Mar 23, 2026 •

edited

Loading