Skip to content

Commit 5fa89e6

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-15019-cube-registry-infer-dead-code
2 parents 02fdc7b + 92b5d7f commit 5fa89e6

868 files changed

Lines changed: 60840 additions & 7557 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
Analytics `$icontains` no longer compiles a `translate()` call on the `sqlite` and `mysql` dialects. On **SQLite** that function does not exist and the statement failed to parse — measured on the engine, not inferred. On **MySQL** the same construct was emitted and its arm is repaired the same way, but nothing was ever executed there: the MySQL arm is asserted as emitted TEXT only, on this face and on `driver-sql`'s alike, so no MySQL parse failure is claimed as measured.
6+
7+
`$icontains` folds ASCII case on both sides of the comparison (#4706 Q1 = A). All three of this package's SQL compilers — the query's own `where` (`NativeSQLStrategy.buildFilterClause`), the ADR-0021 D-C read scope (`compileScopedFilterToSql`) and the `ObjectQLStrategy` echo of that statement — spelled that fold as `translate(col, 'ABC…', 'abc…')` on all four dialect values a compiler can see: `sqlite`, `mysql`, `postgres` and `unknown`, onto which `normalizeSqlDialect` maps everything else, an unset hook and `'oracle'` included. `translate()` is PostgreSQL/Oracle; SQLite has none. Measured on sql.js 1.14.1 (SQLite 3.49.1, the engine `driver-sqlite-wasm` runs), `SELECT translate('ABC','ABC','abc')` answers `no such function: translate` — so this was not a filter that returned the wrong rows, it was a statement the engine refused. On a SQLite datasource, an analytics `where` carrying `$icontains` and an **RLS read scope** carrying it were both unusable.
8+
9+
The fold is now chosen per dialect, on the same construct table the case-exact text family already used, reached through one `fold` flag:
10+
11+
- **SQLite**`lower(col) GLOB lower(?)`. SQLite's `lower()` is ASCII-only (measured: `lower('CAFÉ')` is `cafÉ`), so this is the ruled fold rather than an approximation of it, and it runs.
12+
- **PostgreSQL** and the `unknown` residue — `translate()`, byte-for-byte what those two arms emitted before. Measured set for that word: this package's own suite pins six cells verbatim — `{NativeSQLStrategy, ObjectQLStrategy echo, compileScopedFilterToSql} × {dialect unset, 'postgres'}` for `{name: {$icontains: 'acme'}}`, full emitted SQL and the exact bound params — and the round-1 contract review widened it to **2,721 cells** (2,720 = `{undefined, 'postgres', 'unknown', 'oracle'} × 5 compiler paths × 8 filter shapes × 17 comparands`, plus the bare `{dialect: undefined}` cell), emitted at the merge-base blobs (all five hash-verified) and again at this head: **0 changed cells, 0 error cells**. Outside that set nothing is claimed — no PostgreSQL server was contacted, and on `sqlite` and `mysql` the bytes deliberately changed (340 of 680 cells each, all inside the four `$icontains` shapes).
13+
- **MySQL** — the nested-`REPLACE` fold over `CAST(… AS BINARY)`, matching what `driver-sql` emits for the same operator; the review measured the two faces byte-equal on 60 of 60 MySQL cells. Asserted as text only — no MySQL server is provisionable in the container that wrote this, so that cell is a declared skip, not a claimed pass.
14+
15+
⚠️ Carve-out, stated because it is the surviving half of the defect and not an aside: an `unknown` dialect that is really SQLite is **not** fixed by this change. The residue is reached by four constructions the round-1 contract review drove rather than reasoned — a `SqlDriver` given a **class** client or an unrecognised spelling (`'libsql'`), a host hook answering knex's own `'sqlite3'`, a directly-constructed public `AnalyticsService` with the optional `sqlDialect` omitted, and a `data` service without `getDriverForObject`. For each of them `translate()` still reaches the engine and still fails to parse, on the `where` path, the read scope and the echo alike. No in-repo SQLite driver lands there — `SqliteWasmDriver` and `TursoDriver` both answer `"sqlite"`, measured — so this is an embedder-composition population, not a shipped-driver one. Tracked as #16028.
16+
17+
`$icontains` and the case-sensitive `$contains` family remain two separate constructs on every dialect the compilers accept — collapsing them would give `$contains` back the case fold #4706 Q2 = A took away from it. Measured set for that word: 510 cells (six dialect names — the four values above plus `'oracle'` and an unset hook, which both normalize to `unknown` — × 5 compiler paths × 17 comparands), 0 of them identical between the two families and no `$contains` cell carrying a fold.
18+
19+
⚠️ One deliberate divergence from `driver-sql`, recorded here rather than only in this package's source: `driver-sql`'s own `unknown` arm folds with `LOWER()`, this one keeps `translate()`. Each face keeps the residue it already had, and adopting `LOWER()` here would silently restore on PostgreSQL the Unicode fold #4706 Q1 = A rules out. The pointer exists on this side only; `driver-sql` carries no cross-reference back.
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@objectstack/service-analytics": minor
3+
---
4+
5+
A `min`/`max` over a string-valued field is described as `string`, not `number` (#16098)
6+
7+
The sibling population of the temporal fix. `min` and `max` return a value **of the aggregated field's own type**, so a `min` over a `text` / `select` / `lookup` / `autonumber` column carries a string — and `POST /api/v1/analytics/dataset/query` described every one of those columns as `type: "number"`, exactly as it did for the temporal family before the temporal half landed.
8+
9+
What changed:
10+
11+
- **`measureResultType` now answers `string` for the string-valued field types too**, in the same one table it already answered `time` from. No second mechanism and no new call site: the rule still answers `undefined` for "no correction", and `queryDataset`'s ADR-0021 result-column enrichment still applies it once, downstream of all four producers of the shape.
12+
- **The corrected spelling is `string`**, the `DimensionType` word a `lookup` or `string` DIMENSION column in the same response already carries (`dataset-compiler.dimensionType`). A textual measure spelled `text` would have been a sixth word in a five-word wire vocabulary, leaving every existing consumer branch unreached — the same argument that chose `time` over `datetime`.
13+
- **Membership is composed from `@objectstack/spec`'s own value classes** (`STRING_VALUE_TYPES`, `SINGLE_OPTION_TYPES`, `REFERENCE_VALUE_TYPES`) rather than re-listed, so what the platform says a field type STORES and what this rule says a `min` over it RETURNS cannot drift.
14+
15+
Corrected: `text`, `textarea`, `email`, `url`, `phone`, `password`, `secret`, `markdown`, `html`, `richtext`, `code`, `color`, `signature`, `qrcode`, `select`, `radio`, `lookup`, `master_detail`, `tree`, `user`, `autonumber` — twenty-one members, each verdict read off the two shipped statements of what the type stores (the spec value contract and `driver-sql`'s DDL column switch).
16+
17+
Deliberately NOT corrected, with the measurement recorded rather than a guess shipped as a declaration:
18+
19+
- **`boolean` / `toggle`** — Postgres has no `min(boolean)` at all, SQLite answers `0`/`1` as numbers, and the driver seam has been recorded answering `false`/`true`. Three readings that disagree about whether a value exists and what kind it is. `DimensionType` does carry a `boolean` word, so the correction is spellable; it is not made.
20+
- **The JSON-column classes** (`multiselect` / `checkboxes` / `tags`, `composite` / `repeater` / `record` / `location` / `address` / `vector`, `json`) — no `min` over `jsonb` on Postgres, serialized TEXT on SQLite.
21+
- **The file types** (`image` / `file` / `avatar` / `video` / `audio`) — their stored form is mid-migration under ADR-0104 D3: the value contract already says an opaque `sys_file` id while the DDL still gives them a JSON column.
22+
- **`formula`** — its result type IS declared, on `FieldSchema.returnType`, but that key is not on `AnalyticsServiceConfig.sourceFieldMeta`'s return shape and is itself optional.
23+
- **`summary`** — measured NUMERIC on both shipped statements (the spec's `NUMERIC_VALUE_TYPES`, and `driver-sql`'s `table.float` column), so the `number` it already carried is correct rather than merely unexamined.
24+
25+
Every member of `FieldType` now carries an explicit verdict, pinned by a test that walks the enum: a field type added to the spec fails that pin instead of silently inheriting the flat `number`.
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
"@objectstack/service-analytics": minor
3+
"@objectstack/spec": patch
4+
---
5+
6+
A dataset measure's `fields[].type` stops contradicting the value beside it: a `min`/`max` over a temporal field is described as `time`, not `number` (#15768)
7+
8+
`POST /api/v1/analytics/dataset/query` described **every** measure column as `type: "number"`, including a `min`/`max` over a `date` / `datetime` / `time` field whose value in the same response is an ISO instant. Measured on a real boot (`@objectstack/cli` 17.3.0, SQLite dev datasource):
9+
10+
```json
11+
{"rows":[{"oldest_last_update_at":"2026-07-04T07:00:00.000Z"}],
12+
"fields":[{"name":"oldest_last_update_at","type":"number","label":"Oldest touch","format":"relative"}]}
13+
```
14+
15+
`min` and `max` return a value **of the aggregated field's own type**, so that column carries an instant and the metadata denied it — which is enough on its own to keep a formatter that branches on the declared type from ever reaching a temporal branch.
16+
17+
What changed:
18+
19+
- **The measure column's type is resolved from the authored measure plus the source field's declared type**, in `AnalyticsService.queryDataset`'s ADR-0021 result-column enrichment — the same block that already resolves `label` / `format` / `currency` / `percentScale`, and the one seam every producer of the shape passes through on the way to the route, which relays that method's return verbatim. The rule itself is `measureResultType` in the new `measure-result-type.ts`, so the per-aggregate verdict has one home instead of four copies.
20+
- **The corrected spelling is `time`**, the `DimensionType` word a temporal DIMENSION column in the same response has always carried. A second temporal word in one wire position would have left every existing consumer branch unreached.
21+
- **Only `min` and `max` move.** `count` and `count_distinct` are numeric however temporal the column they read is; `sum` / `avg` over a temporal column are refused by no layer and answered by the backend (an epoch mean on SQLite, an error on Postgres), so there is no single value for a type to describe and none is invented; a derived measure is numeric by construction, because `computeDerived` coerces its operands with `Number()`. Row values are untouched on every path.
22+
- **Tiered "cannot answer, do not block".** A host with no source-field metadata wired, and a measure over a relationship PATH (which the source-field lookup resolves against the base object and therefore cannot answer), both leave the column exactly as the query layer produced it.
23+
24+
`AnalyticsResult.fields[].type` and the `AnalyticsResultResponse` schema now state the vocabulary this position speaks and what each aggregate answers; neither declaration widens — the wire type was, and remains, a string.
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@objectstack/service-analytics": minor
3+
---
4+
5+
A draft-preview `min`/`max` answers the operand's own type instead of `0`, and a preview dimension column is described by its own type
6+
7+
`POST /api/v1/analytics/dataset/query` has two producers of one response: the engine, and — when the request renders the as-if-published world over a pending seed draft (ADR-0037 P3) — `evaluateAnalyticsQueryOverRows`. The second one coerced every aggregate operand with `Number()` and dropped the non-finite ones, so a `min` / `max` over a non-numeric field answered `0`. Measured on one dataset and one row set, with two services differing only in whether a pending seed draft exists:
8+
9+
```
10+
live {"category":"travel","latest_spend":"2026-05-12"}
11+
preview {"category":"travel","latest_spend":0}
12+
```
13+
14+
That is not a mislabelled column: it is a different, wrong answer to the same query, with no refusal and no warning, on the path an author is looking at *while* authoring the dataset.
15+
16+
What changed, per member of the closed `AggregationFunction` vocabulary:
17+
18+
- **`min` / `max` return the winning operand in its own type.** Ordering goes through this file's shared `compare` — so an ISO date orders as a date, a BSON `Date` orders as its instant against wire text, and text orders the way `MIN(text_col)` does on a SQL face — with a numeric arm so a numeric column written as text (`'800'`) still orders numerically. `cross-object-rebucket.ts` settled the identical question for the recombination path: the value these two pick is a value OF the column, so it must come back in the shape the row carried.
19+
- **A group whose operand is null throughout answers `null`, not `0`**`emptyGroupValueFor` (`@objectstack/spec/data`) rules `min` / `max` over nothing unanswerable, and `0` reads as a measurement nobody made.
20+
- **`count_distinct` answers a cardinality again.** Its arm was spelled `countDistinct`, a word no producer mints (`dataset-compiler` copies the spec's `count_distinct` through), so it was unreachable and the measure fell to the numeric default — answering a row count under the author's `count_distinct` name (measured: `3` where the live path says `2`).
21+
- **`count` stays a row count and `sum` / `avg` stay arithmetic.** Counting dates is still counting.
22+
- **`sum` / `avg` over a TEMPORAL operand is deliberately unchanged.** There is no defined answer — the SQL faces do not agree on one either — and refusing an incoherent aggregate/field-type pair is an open decision, not this fix's to invent.
23+
- **A dimension column is typed from the cube dimension**, the same expression both live producers use (`d?.type || 'string'`), so a `date` dataset dimension is `time` on the preview path as it already was on the live one. A MEASURE column keeps the `number` every producer mints; correcting that is the ADR-0021 descriptor pass's one rule, not a second copy here.
24+
25+
Derived measures are untouched: `computeDerived` still coerces with `Number()` and answers `null` for a non-finite operand — but a derived ratio over a temporal `min` / `max` now sees a date instead of the spurious `0`, so it answers `null` on the preview path exactly as it already did on the live one.
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
Analytics `$icontains` no longer compiles a `translate()` call on the `unknown` dialect arm, so a datasource whose dialect nothing answered — which includes SQLite — gets a statement its engine can parse. **Graded `patch`:** no exported type, signature or option changes; the package's own contract for the operator (#4706 Q1 = A, an ASCII-only fold on both sides) is unchanged, and this repairs an arm that could not run rather than adding or retiring behaviour. What moves is emitted SQL text on one arm, measured and enumerated below.
6+
7+
`normalizeSqlDialect` maps **everything it cannot name** onto `unknown`: an unset `sqlDialect` hook, `'oracle'`, `'libsql'`, a `SqlDriver` handed a knex Client **class** rather than a spelling. #15780 left that arm folding with `translate()` and recorded it as "never broken", which was true of the dialects the arm was *pictured* as — mssql and oracle, which have `translate()` — and false of the ones actually routed there. Measured on sql.js 1.14.1 (SQLite 3.49.1, the engine `driver-sqlite-wasm` runs), `SELECT translate('ABC','ABC','abc')` answers `no such function: translate`, so on all three of this package's compilers — the query's own `where` (`NativeSQLStrategy.buildFilterClause`), the ADR-0021 D-C read scope (`compileScopedFilterToSql`) and the `ObjectQLStrategy` echo — the statement failed to **parse**. It reached the client as a 500, not an ADR-0112 refusal. One of the four constructions that land there is a directly-constructed public `AnalyticsService` with its **optional** `sqlDialect` omitted: leaving out an optional field turned a documented operator into a 500.
8+
9+
The `unknown` arm now folds with one nested `REPLACE` per ASCII letter — the chain the MySQL arm already used, minus its `CAST(… AS BINARY)`, so there is one builder and the two arms cannot fold different alphabets. `REPLACE` is the one string function every SQL dialect has, and the domain is the same 26-letter constant, so the fold is ASCII-only **by construction**:
10+
11+
- **PostgreSQL / Oracle-like** — same result set as `translate()`. The chain equals the simultaneous `A`-`Z` map because no step can feed a later one: every replacement writes a lower-case letter and every later step matches an upper-case one. Measured on the engine over **every ASCII code point** plus accented, Greek, Cyrillic and dotted-I probes, required equal to the ASCII-only map exactly.
12+
- **SQLite-like** — it runs. Executed over the shared `FILTER_TEXT_CASES` `$icontains` rows through all three compilers on sql.js: the same row sets the `sqlite` arm is required to answer, including the `CAFÉ`/`café` pair that separates an ASCII fold from a Unicode one.
13+
-**Not `LOWER()`**, which is what `driver-sql`'s own `unknown` arm folds with. `LOWER()` follows the collation, so adopting it would trade this parse failure for **silently wrong rows** on PostgreSQL — the Unicode fold #4706 Q1 = A rules out. ⚠️ Measuring `LOWER()` in this container proves nothing about that: SQLite's `lower()` is ASCII-only and passes the same fixture, which is exactly the trap of letting a green SQLite reading stand in for a PostgreSQL one. No PostgreSQL server was contacted.
14+
15+
**Which cells moved.** The emitted SQL and bound params of `{NativeSQLStrategy, ObjectQLStrategy echo, compileScopedFilterToSql} × {undefined, 'unknown', 'oracle', 'libsql', 'postgres', 'sqlite', 'mysql'} × 5 text operators × 17 comparands` = **1,785 cells**, generated at this head and again with the emitter reverted to its merge-base blob (both legs hash-verified on disk and rebuilt, the marker's presence and absence checked in `dist/`): **204 moved, 1,581 byte-identical, 0 error cells either side.** Every moved cell is `$icontains` on one of the four dialect inputs that normalize to `unknown` (51 each = 17 comparands × 3 compilers). **0 of the 204 changed their bound params** — only the fold's spelling moved, never the escaping or the `ESCAPE` binding. Nothing moved on `postgres`, `sqlite` or `mysql`, and no case-exact operator moved on any dialect input.
16+
17+
⚠️ **The cost, stated rather than left to be found:** the predicate grows from 168 to 1,014 characters on the read scope (233 → 1,079 on the other two). Both constructs are non-sargable scalar expressions over the column, so the plan class is unchanged — what grows is statement text and per-row work, on the arm where the alternative was a statement that did not run.
18+
19+
⚠️ **The residue that remains**, because this arm is a residue and not a dialect: the fold is exact everywhere, but the comparison is `LIKE`, which on a case- or accent-insensitive collation (MySQL/MariaDB arriving here through the `'mariadb'` spelling #11756 deliberately leaves unrecognised; SQL Server) over-matches beyond ASCII. That is the **same** residue this arm's case-exact neighbour already carries and names — not a new one — and on those engines `translate()` did not run at all, so nothing that answered correctly before stops answering.
20+
21+
`SqliteWasmDriver.dialectName` gains a direct pin. It answers `"sqlite"` only through an `isSqlite` override (the base class string-matches `config.client`, and this transport passes a class), that override had **0 direct test hits**, and it is the sole reason no in-repo SQLite driver reaches the arm above. The new pin includes the control: the base class answers `'unknown'` for that very config.

0 commit comments

Comments
 (0)