Skip to content

Commit 3ff7fb7

Browse files
refactor(driver-sql, plugin-security): read the list-comparand split from @objectstack/spec/data (#29660)
<!-- claude-projects-attribution: {"github_login":"os-warren"} --> _Requested by **Warren** · [project thread](https://claude.ai/code/project/chan_01U5RdnAAmkSQmZbqh6FtqZ8?thread=cmsg_01U5RdnAAmkSQmZbqh6FtqZ86whSn5a9tBQC9Cw4hccB47)_ Fixes #29611 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:`driver-sql` 的 `sql-driver.ts` 与 `plugin-security` 的 `rls-check-stored-form.ts` 删掉各自手抄的 `LIST_COMPARAND_OPERATORS`(`$in` / `$nin` / `$between`),改为调用 `@objectstack/spec/data` 导出的 `isListComparandOperator`。 - 为什么改:#29419 已把这条运算符拆分收归 spec 一处;这两份副本是剩下的漂移点。spec 的列表运算符一变,SQL 驱动的成员绑定检查和 RLS 存储形态检查会静默跟丢。 - 风险与回滚:行为不变。改动前已实测两份本地集合与 spec 谓词在 43 个拼写上逐一答案相同;两包现有测试全绿。回滚即撤销本 PR。 - 先看:`packages/plugins/plugin-security/src/rls-check-stored-form.ts`(`storedOperand` 一行替换),`packages/drivers/driver-sql/src/sql-driver.ts`(`assertCompilableComparand` 成员循环一行替换,加导入)。 - 两分钟验证:`git grep -n LIST_COMPARAND_OPERATORS -- packages/drivers/driver-sql packages/plugins/plugin-security`,预期零命中(退出码 1)。 - 需要你做的:无,席位落地。 ## What changes - **`packages/drivers/driver-sql/src/sql-driver.ts`**: `assertCompilableComparand` bind-tests each member of a list operator's array only when `isListComparandOperator(op)` answers true. The module-private `LIST_COMPARAND_OPERATORS` and its docblock are removed. The docblock's reason (a scalar operator handed an array keeps its own "requires a single comparable value" message) moves into the call-site comment. A new import line from `@objectstack/spec/data` carries a comment in the file's existing style, and the `{@link}` in the scalar set's docblock now names the predicate. - **`packages/plugins/plugin-security/src/rls-check-stored-form.ts`**: `storedOperand` asks `isListComparandOperator(op)`, added to the existing `@objectstack/spec/data` import. The module-private copy is removed. The `VALUE_COMPARISON_OPERATORS` docblock now says the list half is the spec's answer. That set is unchanged: it is a different question (which operators compare with the stored value), and the card does not move it. - **Changesets**: `.changeset/29611-driver-sql-list-comparand-predicate.md` (`@objectstack/driver-sql` patch) and `.changeset/29611-plugin-security-list-comparand-predicate.md` (`@objectstack/plugin-security` patch), each with both declaration lines. - No `packages/spec` edit. No `protocol.ts` file is touched. ## Tests - No test changed. Neither package had a test that only reconciled its copy against the spec, so there was nothing to delete. - The existing suites cover both call sites (for example `sql-driver-compile-refusal-seam.test.ts` for the list-member bind refusal) and stay green. - No ablation: the diff changes no tested behaviour, so the equality reading below stands in for one. ## Acceptance notes - **Equality before the swap.** On base `4d8095ba`, a scratch script read each local set from source and compared it with the built spec predicate over 43 spellings: the 19 operators `FieldOperatorsSchema` declares, plus 24 others (the infix spellings `=`, `==`, `!=`, `<>`, `>`, `>=`, `<`, `<=`, `like`, `ilike`; `in`, `nin`, `not in`, `between`; `$and`, `$or`, `$not`, `$field`, `$IN`; the empty string; and `constructor`, `__proto__`, `toString`, `hasOwnProperty`). Both sets answered exactly as `isListComparandOperator` on every spelling, and each holds `$in`, `$nin`, `$between`. The spec export covers every case the local copies handled. The script was not committed. - Both packages already depend on `@objectstack/spec` (`workspace:*`) and import from `@objectstack/spec/data`, so no manifest changes. - `driver-sql` keeps its other local operator sets (`SCALAR_COMPARAND_OPERATORS` with infix spellings, `TEXT_PATTERN_OPERATORS`, `LIKE_PATTERN_OPERATORS`). They are driver-specific and outside this card. ## Verification Readings on head `19e661f7`, which merges `origin/main` `3482f3d8` into the change commit `a7214859` (base `4d8095ba`). Each reading was also taken on `a7214859` before the merge, with the same results except where noted. <details><summary>Tests and typecheck</summary> - Under the verify lock on `19e661f7`: `pnpm install --frozen-lockfile`, then `pnpm exec turbo run build --filter=!@objectstack/docs --concurrency=2`, 117/117 tasks, `VERDICT command-exit 0`. - `pnpm --filter @objectstack/driver-sql --filter @objectstack/plugin-security typecheck`: `VERDICT command-exit 0`. - `pnpm --filter @objectstack/plugin-security test`: 241 files passed, 5203 tests passed, 45 skipped, `VERDICT command-exit 0`. (5202 on `a7214859`; the merge brought a reworked `it.each` table in `default-row-scope-declarations.test.ts`, from #29575.) - `pnpm --filter @objectstack/driver-sql test`: 236 files passed, 12 skipped; 3847 tests passed, 214 skipped, `VERDICT command-exit 0`. - `test:repo` is not run locally (no file of that project changed); it is CI's `Test Core`. </details> <details><summary>Derived gates and lint</summary> - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` on `19e661f7` derived 60 families. All 60 ran, each exit 0, exit codes captured before any pipe. `--ran` reconciliation: "60 derived, 60 run, 0 NOT-MEASURED, 0 UNRUN", exit 0, no stale-tree warning. - On `a7214859` the same derivation gave 61 families (the extra one, `check:pm-changeset-deadline-census`, was retired on `main` by #29577). There, `check:dual-build-cjs-loads` and `check:i18n` first exited 3 (PREREQUISITE NOT MET: no `dist/` outside this diff's closure). After a full build both exited 0, and the reconciliation read 61 of 61 run. That run also warned the tree was 10 commits behind `origin/main`, which is why the branch merged `main` and every reading was taken again. - `pnpm lint` (repo-wide eslint) exit 0 on both heads. </details> Dogfood battery: not run (no `!` in the title and no `(narrowing)` arm). ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (No spec change.) - [x] 2. Every name I added uses the casing for its kind. (No new names.) - [x] 3. No formatter ran; `pnpm lint` passes (local run, exit 0). - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: not touched, nothing to regenerate. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. --- _Generated by [Claude Code](https://claude.ai/code/session_01JQwxbYnNFzqeb7KM8Lpd7j)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent a2f77f7 commit 3ff7fb7

4 files changed

Lines changed: 37 additions & 20 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/driver-sql': patch
3+
---
4+
5+
refactor(driver-sql): the comparand bind test reads the list operators from the spec
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
The SQL driver bind-tests each member of a `$in`, `$nin` or `$between` list as a comparand in its own right. It now asks `isListComparandOperator` from `@objectstack/spec/data` which operators take a list, instead of keeping its own copy of `$in` / `$nin` / `$between`. The set is the same, so no filter compiles or is refused differently. A later change to the spec's list operators now reaches this driver without a second edit.
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
refactor(plugin-security): the RLS stored-form check reads the list operators from the spec
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
The RLS check puts a value comparand into its column's storage form, and a `$in`, `$nin` or `$between` list member by member. It now asks `isListComparandOperator` from `@objectstack/spec/data` which operators take a list, instead of keeping its own copy of `$in` / `$nin` / `$between`. The set is the same, so no policy filter is judged differently. A later change to the spec's list operators now reaches this check without a second edit.

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

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,11 @@ import {
7272
isAcceptedFilterComparand,
7373
ACCEPTED_FILTER_COMPARAND_TYPES_SENTENCE,
7474
} from '@objectstack/spec/data';
75+
// [#29611] The operator split's list half (`$in` / `$nin` / `$between`): the
76+
// operators whose comparand is a LIST, so each member is bind-tested on its own
77+
// (see {@link assertCompilableComparand}). Asked of the spec, the one source the
78+
// comparand faces read, so this driver keeps no copy to drift from it.
79+
import { isListComparandOperator } from '@objectstack/spec/data';
7580
// [#7536] `$like`/`$ilike`'s pattern language, defined once in the spec: the
7681
// dangling-escape gate every face refuses on, and the LIKE→GLOB translation the
7782
// SQLite dialects need because GLOB is the only case-exact pattern operator
@@ -2487,7 +2492,7 @@ const CROSS_FIELD_COMPARISON_OPERATORS: ReadonlySet<string> = new Set([
24872492
* their own `whereIn` / `whereBetween` arms. Only their MEMBERS are inspected —
24882493
* for `$field` (#5041) and, since #5234, for bindability — never their arity:
24892494
* the existing descriptive `$between` refusal stays the one that answers a
2490-
* malformed range. See {@link LIST_COMPARAND_OPERATORS}.
2495+
* malformed range. See {@link isListComparandOperator}.
24912496
*/
24922497
const SCALAR_COMPARAND_OPERATORS: ReadonlySet<string> = new Set([
24932498
'$eq', '$ne', '$gt', '$gte', '$lt', '$lte',
@@ -2569,17 +2574,6 @@ const TEXT_OPERATORS: ReadonlySet<string> = new Set([
25692574
...LIKE_PATTERN_OPERATORS,
25702575
]);
25712576

2572-
/**
2573-
* [#5234] Operators for which an ARRAY is the legitimate comparand, so it is
2574-
* each MEMBER that must be individually compilable.
2575-
*
2576-
* Scoping the member scan to these three is what keeps a scalar operator that
2577-
* received an array answering with its own message ("requires a single
2578-
* comparable value") instead of reporting the first bad member of a list it
2579-
* should never have been given.
2580-
*/
2581-
const LIST_COMPARAND_OPERATORS: ReadonlySet<string> = new Set(['$in', '$nin', '$between']);
2582-
25832577
/**
25842578
* [#5234] Does this value have a faithful rendering as the text of a LIKE
25852579
* pattern?
@@ -2687,10 +2681,13 @@ function assertCompilableComparand(
26872681
if (nowAnchorOf(member) !== null) throw nowAnchorPositionError(field, op, member, index);
26882682
// [#5234] Every member of a list operator's array is a comparand in its
26892683
// own right and gets the same bind test the whole comparand gets. Scoped
2690-
// to the operators for which an array is legitimate, so a scalar operator
2691-
// handed an array keeps answering with its own message below.
2684+
// to the operators for which an array is legitimate (the spec's list
2685+
// half, {@link isListComparandOperator}), so a scalar operator handed an
2686+
// array keeps answering with its own message ("requires a single
2687+
// comparable value") instead of reporting the first bad member of a list
2688+
// it should never have been given.
26922689
// [#20039] The member when it is an object, else the list that holds it.
2693-
if (LIST_COMPARAND_OPERATORS.has(op) && !isBindableComparand(member)) {
2690+
if (isListComparandOperator(op) && !isBindableComparand(member)) {
26942691
throw unbindableListMemberError(field, op, member, index, refusalSubtree(member, value));
26952692
}
26962693
}

‎packages/plugins/plugin-security/src/rls-check-stored-form.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ import {
163163
INSTANT_TYPES,
164164
STRUCTURED_JSON_TYPES,
165165
filterSubtreeProvenanceOf,
166+
isListComparandOperator,
166167
isMultiValueField,
167168
markFilterSubtreeProvenance,
168169
} from '@objectstack/spec/data';
@@ -179,15 +180,14 @@ const NO_MULTI_VALUE_COLUMNS: DeclaredMultiValueColumns = new Set();
179180
/**
180181
* The operators whose comparand is a VALUE of the column, compared with the
181182
* stored value. The drivers put these comparands into the column's storage
182-
* form; every other operator's operand is left as written.
183+
* form; every other operator's operand is left as written. Which of them take
184+
* a LIST of values is the spec's answer ({@link isListComparandOperator}), not
185+
* a copy kept here.
183186
*/
184187
const VALUE_COMPARISON_OPERATORS: ReadonlySet<string> = new Set([
185188
'$eq', '$ne', '$gt', '$gte', '$lt', '$lte', '$in', '$nin', '$between',
186189
]);
187190

188-
/** The value-comparison operators whose comparand is a LIST of values. */
189-
const LIST_COMPARAND_OPERATORS: ReadonlySet<string> = new Set(['$in', '$nin', '$between']);
190-
191191
/** The storage rule a declared type takes, or `undefined` for a non-temporal type. */
192192
function temporalKindOfDeclaredType(type: string): TemporalComparandKind | undefined {
193193
if (CALENDAR_DATE_TYPES.has(type)) return 'date';
@@ -412,7 +412,7 @@ function storedComparand(value: unknown, kind: TemporalComparandKind): unknown {
412412

413413
/** A value-comparison operand in its column's storage form, a list member by member. */
414414
function storedOperand(op: string, value: unknown, kind: TemporalComparandKind): unknown {
415-
if (!LIST_COMPARAND_OPERATORS.has(op) || !Array.isArray(value)) return storedComparand(value, kind);
415+
if (!isListComparandOperator(op) || !Array.isArray(value)) return storedComparand(value, kind);
416416
let copy: unknown[] | undefined;
417417
value.forEach((member, index) => {
418418
const stored = storedComparand(member, kind);

0 commit comments

Comments
 (0)