fix(driver-memory): analytics 面不再把比较数往返成 string[](布尔取 0 行、null 取全表)(#5373) - #5431
Merged
Merged
Conversation
…s through string[] (#5373) `MemoryAnalyticsService` lowered `AnalyticsQuery.where` into a cube-style `{member, operator, values}` list whose `values` was `string[]`, so every comparand made a JS value → string → JS value round trip. That round trip is lossy for anything not already a string, and both of the issue's symptoms are the one root cause: - `stringifyForCube(true)` → `'1'` → `coerceFilterValue('1')` → the NUMBER 1 (the `/^-?\d+$/` arm wins), compared against a stored `true`. mingo compares cross-type as never-equal, so `{is_active: true}` matched zero rows. - `flattenFilterCondition` opened with `if (raw == null) continue`, so `{closed_at: null}` produced no cube entry at all and the predicate vanished. Fewer constraints means MORE rows: the query widened to the full table. This is the #3948 direction and the more dangerous half — a widened chart looks exactly like a working chart. Route B of the issue's three: stop round-tripping. `values` is `unknown[]`; stringification happens only at the `generateSql` exit, where a SQL literal is genuinely needed. A/C were rejected because A layers a tag on an encoding the issue already calls suspect, and C would refuse `{is_active: true, stage: {$nin: ['lost']}}` — `AnalyticsQuerySchema.where`'s own docstring example. B is affordable because the triple is a purely INTERNAL intermediate. Verified rather than assumed before building on it: `normalizeFilters`, `flattenFilterCondition`, `stringifyForCube`, `coerceFilterValue` and `toSqlLiteral` are all private and referenced only in this file; `index.ts` exports only the class; `IAnalyticsService` exposes no such shape; and the API layer actively REJECTS a `{member, operator, values}` array on the wire (`spec/src/api/analytics.test.ts`, `runtime/src/http-dispatcher.test.ts` both assert the rejection). Zero spec bytes touched. The encoding could not simply be made lossless: its own justification for `'1'`/`'0'` ("downstream consumers expecting SQLite-style numeric booleans") is true for the SQL exit and false for the in-memory one, and both exits shared it. Both exits are fixed, because a fix that satisfied mingo while emitting SQL meaning something else would only move the loss. `toSqlLiteral` now takes the real value instead of guessing a type back out of text — a TEXT `'100'` is quoted where it used to emit `code = 100` — and a null comparand becomes `IS NULL` / `IS NOT NULL` rather than SQL's never-true `= NULL`. Temporal comparands still convert, now via the driver's own storage-form rule (new narrow `filterComparandStorageForm`, keyed on the declared field kind, #4047) instead of an ad-hoc `toISOString()`, so a `Date` still meets a declared `datetime` column and no second derivation of that rule appears in this face (#5240). The issue's unverified third symptom is REAL and fixed by the same change: `{code: '100'}` against a TEXT column storing `'100'` round-tripped to the number 100 and matched zero rows. Measured, along with two more of the same root cause the issue did not list — `{is_active: {$ne: true}}` and `{closed_at: {$ne: null}}` each returned the whole table. Measured on the issue's 3-row fixture, analytics vs `find()`: | where | before | after | find() | |---|---|---|---| | `{is_active: true}` | 0 | 2 | 2 | | `{is_active: false}` | 0 | 1 | 1 | | `{closed_at: null}` | 3 | 2 | 2 | | `{code: '100'}` | 0 | 2 | 2 | | `{is_active: {$ne: true}}` | 3 | 1 | 1 | | `{closed_at: {$ne: null}}` | 3 | 1 | 1 | Tests go in the shared conformance file beside the #5324/#5345 shape table rather than a suite of their own. `FILTER_LOGIC_CASES` varies filter SHAPE over an all-string fixture — deliberately, so nothing in it is about coercion — which is exactly why every case stayed green through this defect. The new block varies comparand TYPE and holds the same invariant: agree with `find()`, or refuse. Reverting only the source change fails 11 of the new assertions, across both exits. Out of scope, not fixed here: #5374 (`$notContains` → bare mingo `{$not: 'x'}`), held to follow serially since its call site is the value-compilation point this PR changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pbu27iNUfQCHeuS551Rqo7
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 9 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
August 5, 2026 11:23
This was referenced Aug 5, 2026
Merged
akarma-synetal
pushed a commit
to akarma-synetal/framework
that referenced
this pull request
Aug 6, 2026
…dicate that excludes rows (objectstack-ai#5374) (objectstack-ai#5445) `MemoryAnalyticsService` mapped each cube operator to the NAME of a mingo operator, and the call site filled that name in as `matchStage[field] = {[name]: comparand}`. That shape can express "compare this field to this value" and nothing else, so the two operators that need to WRAP their comparand were pushed through it anyway. `notContains` -> `'$not'` became `{name: {$not: 'et'}}`. mingo's `$not` takes a regex or an operator expression; handed a bare scalar it constrains nothing, so the predicate was emitted, appeared in the pipeline, and passed the whole table: 3 rows where `find()` returns 2. A predicate that is emitted and inert is indistinguishable from a working one at the author's end — the same amplifying direction as objectstack-ai#3948, arrived at a third way. objectstack-ai#5345 ruled on operators with NO mapping, objectstack-ai#5373 on the comparand ENCODING; this is a mapping pointing at the wrong target, and objectstack-ai#5431 did not shrink it (that call site now receives a real value, which is orthogonal to what the operator layer does with it). Route: let the map return a STRUCTURE rather than an operator name, which the issue prefers and the code supports cleanly. `CUBE_OPERATOR_TO_MONGO_PREDICATE` holds a builder per operator that returns the whole `{$op: …}` object, so `notContains` can say `{$not: {$regex: …}}` and the CLASS of "this operator needs a structure and the table can only hold a name" is gone rather than this one instance. `$in`/`$nin`/`$lte`/`$exists`, which the call site had grown an `if` chain for, are ordinary rows in that table now. Measured on the issue's 3-row fixture, analytics vs `find()`: | where | before | after | find() | |----------------------------------|--------|-------|--------| | {name:{$notContains:'et'}} | 3 | 2 | 2 | | {name:{$notContains:'a'}} | 3 | 0 | 0 | | {name:{$contains:'a.p'}} | 1 | 0 | 0 | | {name:{$contains:'ALPHA'}} | 0 | 1 | 1 | | {name:{$notContains:'ALPHA'}} | 3 | 2 | 2 | | {made_at:{$contains:'<full ISO>'}}| 1 | 0 | 0 | | {code:{$in:[]}} | 3 | 0 | 0 | | {code:[]} | 3 | 0 | 0 | Three more defects at the same call site fall inside this fix and are closed with it, because writing a correct `notContains` requires settling each: - `contains` was the right operator with the comparand handed in RAW, so it was neither escaped (`.` matched any character) nor case-folded, while the live path escapes and matches `/…/i`. Leaving that would have made the two non-complementary in a new way — `alpha` would be in BOTH answers. The rule is now borrowed from the driver (new narrow `filterSubstringPattern`, alongside `filterComparandStorageForm`) rather than re-derived, per objectstack-ai#5240. - An operand that is NOT a comparand went through the storage-form conversion anyway, so on a declared `datetime` column a `$contains` PATTERN was rewritten into canonical form and then matched rows `find()` does not match. The builder input carries both lists, the same split `normalizeFieldOperators` makes (objectstack-ai#4047). - The call site's `values.length > 0` guard meant an empty `$in` emitted no predicate at all and widened to the whole table. A list operator taking the whole list has nothing to guard. The two items the issue flagged as unmeasured, settled: - `'inDateRange': '$gte'` compiles to NOTHING today — no `MONGO_TO_CUBE_OPERATOR` entry lowers to that name, `timeDimensions` never reaches this function, and both exits consume only `normalizeFilters` output. Dead, and wrong if it ever had been reached (a one-ended `>=` for a two-ended range, which its own comment conceded). Deleted, with the dead-and-inverted `'notSet': '$exists'` beside it. - `opMap[operator] || '$eq'` is unreachable for the same reason — but only until someone widens the vocabulary, which objectstack-ai#5345 deliberately made a one-line edit to `MONGO_TO_CUBE_OPERATOR`. So it is not merely deleted: that table is `as const`, the predicate table is keyed by the operator union derived from it, and the widening edit now FAILS TO COMPILE until the predicate exists. The remaining throw is a totality floor, not a fallback. Tests go in the shared conformance file beside the objectstack-ai#5345 shape table and the objectstack-ai#5373 comparand-type table, as a third axis with the same invariant: agree with `find()`, or refuse. Plus the "declared = enforced" half — every operator `ANALYTICS_FILTER_CAPABILITIES` declares is driven through both faces and must agree, with a probe that must exclude at least one row, so an operator added to the vocabulary without a working lowering fails here instead of shipping a quietly wrong number. Reverting only the source change fails 14 of the new assertions. Out of scope, filed not fixed: objectstack-ai#5440 (two operators on one field clobber each other — the `$match` assembly layer, still broken after this), objectstack-ai#5442 (`flattenFilterCondition` spreads an array comparand for every operator), objectstack-ai#5444 (the `generateSql` exit emits `LIKE 'et'` with no `%` wildcards — filed as a sub-issue of objectstack-ai#5433, whose completion scope it falls inside). `operatorToSql` and `generateSql` are untouched. Claude-Session: https://claude.ai/code/session_01Pbu27iNUfQCHeuS551Rqo7 Co-authored-by: Claude <noreply@anthropic.com>
akarma-synetal
pushed a commit
to akarma-synetal/framework
that referenced
this pull request
Aug 6, 2026
…in 滞后、死代码删除复核 (objectstack-ai#5513) (objectstack-ai#5645) 2026-08-05 跑完一整条 filter 缺陷链(objectstack-ai#5363 / objectstack-ai#5366 / objectstack-ai#5368 / objectstack-ai#5375 / objectstack-ai#5431 / objectstack-ai#5445, cloud#1117)后回看,六处在那一轮真实咬过人或真实救过场的规程,SKILL 里没有对应条目。 六条各落在 issue 指定的节内,**纯增补**:111 行插入、0 行删除,既有条目(objectstack-ai#5501 的接力 模式、objectstack-ai#5522 的座位模型、objectstack-ai#5630 的 assertEngineDeleteDispatch 条款)一字未动。 落点与要点: 1. **Multi-repo,rule 2 之后**「pin 滞后」——`Blocked-by:` 只保证上游已合并,姊妹仓还有 第二个读数:本仓 pin 是否覆盖那个 commit。cloud#1116 的裁决落于 framework objectstack-ai#5368 (`9c5abf4e9`),而 cloud 的 `.objectstack-sha` 未覆盖它,于是 `TursoDriver` 有一个 方向反了的分叉窗口(fail-closed 一侧先到)。规程:派发前核祖先关系;未覆盖则 dev 在 PR 正文留档窗口与方向,⛔ pin bump 不做 rider。 2. **step 3** 末「阻塞解除后重新定价」—— 前一单合入会改变后一单的成本模型,方向不止一个 (本轮变便宜、没变、成本估计过期各有实例)。两个动作配对:派发前一单时带必答项 「你的改动是否让 #X 变简单 / 变难 / 不必要 / 无影响」,派发被延后那单前用该回答重读 其选项与成本估计。 3. **step 5** 派发令「多面组件的测试落点」—— 同一契约 ≥2 实现面时,新用例进共享一致性 覆盖而非独立文件(原话照录)。附 objectstack-ai#5375 / objectstack-ai#5431 / objectstack-ai#5445 三条正交轴共用一条不变量。 4. **step 7 清单**「收益穿过它必经的那道边界之后还在吗」—— 判据是价值主张是否依赖下游 如实转发;实例即 objectstack-ai#5423(4xx 直通曾整条替换 ≥500 字符正文,`code` 到了正文没到)。 5. **step 7 清单**「死代码删除的复核」——「这是死代码」是断言而非能从 diff 读出的事实, PM 在 origin/main 独立核一次引用面再 ACCEPT(查法用 Operational notes 6:notes 6 说 怎么查不假阴性,本条说什么时候必须查)。 6. **step 8** 升级门槛之后「带前提的裁决」—— 分歧关键是可被代码证伪的事实时,第三档 = 裁决 + 前提验证要求 + 「前提不成立报 fork,不许硬做也不许悄悄改选」禁令,三件缺一 不可;缺第 3 条即退化为无人裁决且无读数显示。 实施时两处核实结果与 issue 正文不同,成文按核实后的事实写: - issue 的附带论断「没有任何闸门在量这个 pin 滞后」**不成立** —— cloud 的 `scripts/check-pin-staleness.sh`(test.yml 以 `continue-on-error` 跑)每次 CI 都报两个 pin 各落后 main 多少 commit,advisory 是**有意设计**(`--max-behind` 需显式传)。它答 的是「落后多少」,不是「是否覆盖我这条裁决 commit」;成文因此指向该脚本,并只把后一个 问题留给派发前的祖先判断。据此**未**另立「无闸门」的发现单。 - 第 4 条的 rest-server 缺陷本身已由 objectstack-ai#5423 按「截断而非替换」修掉,成文改用过去时并注明, 以免后来的读者去找一个已不存在的活 bug;该条要补的是**复核清单的缺口**,与代码是否已修 无关。 第 1 / 3 条按 issue「未验证的部分」的克制写入适用判据(前后单共用同一契约或数据表示; 组件对同一契约有 ≥2 实现面),形态迥异的批次(纯 UI、纯文档)明确不强加。 验证:`node scripts/check-nul-bytes.mjs --self-test` + 全仓扫描绿(48 断言 / 5537 文件); 改动文件自扫 `grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]'` 零命中,并用邻近词反查证伪 「扫描器坏了」;`check:docs-audit-scope` 绿;markdown 结构核对(强调标记成对、代码围栏 16 个偶数、嵌套围栏缩进对齐)。 Claude-Session: https://claude.ai/code/session_01GX3sL71LFq8m2usg6VqTSE Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: os-zhuang <hr@objectstack.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5373
根因:一个编码,两个出口
MemoryAnalyticsService把AnalyticsQuery.where降级成 cube 风格的{member, operator, values}列表,而values是string[]—— 所以每个比较数都要走一遍 JS 值 → 字符串 → JS 值的往返。这个往返对任何本来就不是字符串的类型都是有损的,
issue 里的两个症状是同一个根因:
stringifyForCube(true)→'1'→coerceFilterValue('1')→ 数字1(
/^-?\d+$/先命中),而记录里存的是true。mingo 跨类型比较恒不等,所以{is_active: true}一行都不匹配。null:flattenFilterCondition开头的if (raw == null) continue让{closed_at: null}连一条 cube 条目都不产生,谓词整条消失。少一个约束 = 多返回行,查询放大到全表。这是 A filter with an operator outside VALID_AST_OPERATORS is silently dropped, not rejected — single-condition views return unfiltered results #3948 立规矩针对的方向,也是两者中更危险的那个 ——
空图表有人会发现,而被放大的图表和正常图表长得一模一样。
关键在于这个编码修不好:注释里给布尔转
'1'/'0'的理由(「downstream consumers expecting SQLite-style numeric booleans」)对生成 SQL 的那一半
成立,对 in-memory 那一半不成立,而两半共用同一个编码。不存在一种
true的字符串写法能同时满足
WHERE is_active = ?和 mingo 对存储布尔值的$eq。采用 Route B:不再往返
values内部改成unknown[],比较数保持作者写下的样子,只在generateSql这个真正需要 SQL 字面量的出口才字符串化。
不选 A/C 的理由:A 是在 issue 自己称为可疑的编码之上再加一层标记;C 会拒掉
{is_active: true, stage: {$nin: ['lost']}}—— 那正是AnalyticsQuerySchema.where自己的 docstring 示例。
前提已实测验证,不是假设
B 成立的前提是这个三元组现在是纯内部中间表示。动手前先验证了这一点:
normalizeFilters/flattenFilterCondition/stringifyForCube/coerceFilterValue/toSqlLiteralprivate,仅在本文件内引用index.ts导出MemoryAnalyticsService类和MemoryAnalyticsConfigIAnalyticsService契约AnalyticsQuery/AnalyticsResult,不含该三元组packages/specspec/src/api/analytics.test.ts与runtime/src/http-dispatcher.test.ts各有一条测试,断言 cube 风格的filters数组会被拒收所以本 PR 未触碰一个 spec 字节。
service-analytics里同名的filter-normalizer.ts是另一个包的独立实现,按 scope 围栏未动。
两个出口都改了
修法如果只让 mingo 满意、却让 SQL 表达另一件事,那只是把 bug 挪了个地方,所以两个出口
一起钉:
toSqlLiteral现在拿到真值,不必再从文本里猜类型 —— TEXT 列的'100'会加引号,以前它和数字
100长得一样,两者都不加引号地输出成code = 100;null比较数编译成IS NULL/IS NOT NULL,而不是 SQL 里恒不为真的= NULL(本次修复前是根本不输出这个子句,也就是放大方向);
1/0—— 那个理由对这一半一直是对的,只是不该被另一半共用。时间类比较数仍然转换,但改为借用 driver 自己的存储形式规则(新增窄接口
filterComparandStorageForm,按声明的字段种类分派,#4047),而不是就地再写一个toISOString()。在本面内重新推导一遍该规则,正是 #5240 裁定要关掉的分叉。实测(issue 的 3 行固定数据,analytics 面 vs 同一 driver 的
find())wherefind(){is_active: true}{is_active: false}{closed_at: null}{code: '100'}(TEXT 列){is_active: {$ne: true}}{closed_at: {$ne: null}}issue 标注为未验证的第三个症状:已实测,是真的,且是同一根因、同一处修复覆盖 ——
TEXT 列存
'100'、比较数写'100',往返成数字100后取到 0 行;generateSql同时输出了不加引号的
code = 100。顺带测出 issue 未列出的另外两条同根因放大:{is_active: {$ne: true}}与{closed_at: {$ne: null}}都返回全表。测试放在共享一致性覆盖里
新用例进的是 #5375 刚扩到 analytics 面的那个共享文件,而不是独立测试文件。原因:
FILTER_LOGIC_CASES变化的是过滤器形状,而它的固定数据每一列都是字符串(有意为之,好让其中没有一条是关于类型强转的)—— 这恰恰是它在本缺陷下全绿的原因。
新增的块变化的是比较数类型,守同一条不变量:与
find()给出相同的行集,或者拒收。回归证明(按 #5375 的做法,只回退源码改动、保留测试):
失败方向与 issue 描述一致:布尔/字符串是空集,
null/$ne是全表;两个出口都失败。恢复源码后
Tests 67 passed (67)。验证
vitest run(driver-memory 全包):491 passed (17 files)pnpm --filter @objectstack/driver-memory typecheck:干净通过eslint --no-inline-config(三个改动文件):无输出范围外
$notContains编译成裸 mingo{$not: 'x'},该谓词不约束任何行 —— 结果被放大到全表 #5374($notContains→ 裸 mingo{$not: 'x'})按要求未动。说明一下:本 PR没有让它变简单,也没有顺带修好它 —— 它的调用点
matchStage[fieldPath] = {$not: coerced[0]}现在拿到的是真值而不是字符串,但{$not: 'x'}约束不了任何东西这个问题在算子层,与比较数编码无关,仍需按原样单独修。generateSql的算子层缺陷(与本 PR 的比较数编码无关,修复前后都存在):operatorToSql没有in/notIn/set/notSet的映射,回退到=,且只取values[0]—— 所以{code: {$in: ['100','200']}}输出WHERE code = '100'。形状与 driver-memory 的 analytics 面静默丢弃大半个 filter:
$or/$not整条丢,$between/$startsWith/$null/$regex因无 cube 映射而丢 —— 聚合结果被放大 #5345/driver-memory analytics 面的$notContains编译成裸 mingo{$not: 'x'},该谓词不约束任何行 —— 结果被放大到全表 #5374 同类,已另行记录,未在本 PR 修。Generated by Claude Code