Skip to content

Commit c637387

Browse files
os-zhuangclaude
andauthored
fix(service-analytics): recoverNumber 只还原规范数字串 —— '007' / '1.50' 保持字符串 (#5528) (#5547)
* fix(service-analytics): only a canonical numeric spelling is a number, so '007' / '1.50' stay strings (#5528) An analytics `where` round-trips every comparand through `values: string[]` — `stringifyForCube` out, `coerceFilterValueForSql` / `coerceFilterValueForObjectQL` back — and the decoder decided "this is a number" from the string's SHAPE alone (`/^-?\d+(\.\d+)?$/`), which cannot tell a stringified number from a string the author wrote. Measured on main, cube `orders` / TEXT column `code`: `'007'` bound `7`, `'0912'` bound `912`, `'1.50'` bound `1.5` — on BOTH consumers, the raw-SQL bind and the engine comparand. The failure was silent and MIS-TARGETED rather than empty: SQLite applies the text column's affinity to the integer bind, so a widget filtered on order number `'007'` returned the row storing `'7'`. On Postgres the same query is a `text = integer` type error; on the engine path the strict comparison matched nothing (measured: 0 rows). Zero-padded and trailing-zero strings are ordinary business shapes — order numbers, SKUs, dialling codes, postcodes, prices. Recovery is now limited to a number's own canonical spelling (`String(Number(s)) === s`): a comparand that really was a number arrives as `String(n)` by construction and still round-trips (`7` → `'7'` → `7`), while a string `Number()` would rewrite cannot have come from a number and stays the author's string. The shape regex still runs FIRST, so the change can only remove recoveries — `'1e3'`, `'1e+21'`, `'+7'`, `' 7'`, `'0x10'`, `'Infinity'`, `'NaN'` were strings before and are strings after. This is the direction ADR-0053 D-A2 already set for this function: last-resort textual recovery behind the driver-backed `coerceTemporalFilterValue` hook. Stopgap by design, and named as one in the TSDoc and the tests: `values: string[]` still has no escape, so the author strings `'null'` / `'true'` / `'false'` still collide with the tokens the encoder writes for the real values. That is #5526's root cause (tagged values, or an `unknown[]` internal representation), pinned here as UNCHANGED so it is not mistaken for fixed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BWS4heBoAitLmzCLhcYdbK * style(service-analytics): fix two stray indents in the #5528 test file Left over from dropping three `eslint-disable-next-line @typescript-eslint/no-explicit-any` comments: that rule is not defined in this repo's eslint config, so the disables themselves were errors ("Definition for rule ... was not found"). The sibling `filter-operator-coverage.test.ts` uses the same `let db: any` harness with no disable comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BWS4heBoAitLmzCLhcYdbK --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 52caf23 commit c637387

3 files changed

Lines changed: 512 additions & 6 deletions

File tree

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
fix(service-analytics): only a canonical numeric spelling is recovered as a number, so `'007'` / `'1.50'` stay strings (#5528)
6+
7+
An analytics `where` round-trips every comparand through the internal
8+
`values: string[]` form — `stringifyForCube` on the way out, and
9+
`coerceFilterValueForSql` / `coerceFilterValueForObjectQL` on the way back. The
10+
decoder decided "this is a number" from the string's **shape** alone
11+
(`/^-?\d+(\.\d+)?$/`), which cannot distinguish a number that was stringified on
12+
the way out from a string the author actually wrote.
13+
14+
Measured before the fix, on cube `orders` / TEXT column `code`:
15+
16+
| author's `where` | leaf `values` | SQL bind | engine comparand |
17+
|---|---|---|---|
18+
| `{code: {$eq: '007'}}` | `["007"]` | `7` | `7` |
19+
| `{code: {$eq: '0912'}}` | `["0912"]` | `912` | `912` |
20+
| `{code: {$eq: '1.50'}}` | `["1.50"]` | `1.5` | `1.5` |
21+
22+
Both consumers were affected: the raw-SQL bind in `NativeSQLStrategy` and the
23+
comparand handed to the ObjectQL aggregate engine.
24+
25+
The failure was **silent and mis-targeted, not empty**. Against a text column
26+
SQLite applies the column's affinity to the integer bind, so a widget filtered on
27+
order number `'007'` returned the row storing `'7'` — a different row, with no
28+
error to read; on Postgres the same query is a `text = integer` type error, and on
29+
the engine path the strict comparison simply matched nothing (measured: 0 rows).
30+
Zero-padded and trailing-zero strings are ordinary business shapes — order
31+
numbers, work orders, SKUs, dialling codes, postcodes, `'1.50'` prices.
32+
33+
Recovery is now limited to a number's **own canonical spelling**
34+
(`String(Number(s)) === s`):
35+
36+
- a comparand that really was a number is `String(n)` by construction, so it
37+
still round-trips — `7``'7'``7`, `1.5``'1.5'``1.5`, `-3``-3`;
38+
- a string `Number()` would rewrite — `'007'`, `'0912'`, `'1.50'`, `'1.0'`,
39+
`'-0'`, or more digits than a double holds — cannot have come from a number, so
40+
it stays the string the author wrote.
41+
42+
The narrowing can only ever **remove** recoveries: the shape regex still runs
43+
first, so `'1e3'`, `'1e+21'`, `'+7'`, `' 7'`, `'0x10'`, `'Infinity'` and `'NaN'`
44+
were strings before this change and are strings after it. This also aligns with
45+
ADR-0053 D-A2, which demoted this textual type re-derivation to a last resort
46+
behind the driver-backed `coerceTemporalFilterValue` hook.
47+
48+
**Stopgap, and named as one.** `values: string[]` still has no escape, so the
49+
author strings `'null'` / `'true'` / `'false'` still collide with the tokens the
50+
encoder writes for the real `null` and booleans. Making the round trip lossless —
51+
tagged values, or an `unknown[]` internal representation — is #5526; the
52+
collision is pinned as unchanged in
53+
`src/__tests__/filter-value-canonical-number.test.ts` so it is not mistaken for
54+
fixed.

0 commit comments

Comments
 (0)