Skip to content

Commit bf37b99

Browse files
fix(service-analytics): on SQLite, the text operators compare the whole stored value and comparand, not only up to their first U+0000 (#20025) (#20067)
Fixes #20025 Clause-②: no ## What this changes `service-analytics` compiles its own SQL for the text operators, in `text-match-sql.ts` `textMatchPredicateSql`, and three faces call it with `dialect: 'sqlite'`: - `compileScopedFilterToSql` (`read-scope-sql.ts`), the read scope, reached directly and through both strategies' scope merge; - `NativeSQLStrategy.buildFilterClause`, the executed `where`; - `ObjectQLStrategy.buildFilterClauseSql`, the `/analytics/sql` echo, which its caller runs on the same database. Its `sqlite` arm emitted `GLOB` for every shape. SQLite's `glob()` reads its pattern AND the stored value as C strings, so each is cut at its first U+0000. `driver-sql` left that construct in #19999 (the comparand's cut) and #20024 (the stored value's cut); this package re-emits the driver's construct table rather than importing it, and its copy had kept `GLOB`. The `sqlite` arm now mirrors `driver-sql`'s `textMatchPredicate` / `sqliteLengthAwareTextMatch` on `main`, cell for cell: | shape | SQLite predicate | |---|---| | `contains` (`$contains`, `$notContains`, `$icontains`) | `instr(col, ?) > 0` | | `ends` (`$endsWith`), non-empty comparand | `coalesce(substr(CAST(col AS BLOB), -length(CAST(? AS BLOB))), CAST(col AS BLOB)) = CAST(? AS BLOB)` | | `ends`, empty comparand | `instr(col, ?) > 0` | | `starts` (`$startsWith`), comparand without U+0000 | `col GLOB ?`, unchanged byte for byte | | `starts`, comparand holding U+0000 | `instr(col, ?) = 1` | - This is the wider split the card's suggested shape predates. `glob()` also cuts the stored value, so every `contains` / `ends` comparand moves, not only a comparand holding U+0000. Only `$startsWith` without U+0000 stays on `GLOB`, because neither cut can change that answer and it is the one shape an index serves. - The fold is still `lower()` on both sides, ASCII-only (#4706 Q1 = A). The four case-exact operators stay case-sensitive (Q2 = A); `instr()` and the BLOB comparison are byte-wise. - The negation is `NOT (…)`, which is NULL for a NULL value, as `NOT GLOB` was. So the NULL-safe `$notContains` wrapper and the `$not` totalisation compose unchanged (#5298). - Every comparand refusal, and the non-text-column constant, still runs before the text arm. Neither door is touched. - The suffix arm binds its comparand twice, in placeholder order, through the existing `TextMatchBind`. The `LIKE` arms already bind two values, so none of the three call sites changes. - Nothing is escaped on the new constructs. `instr()` and the BLOB suffix have no pattern language, so `*`, `?` and `[` bind as written. - `text-match-sql.ts`'s header now describes the `sqlite` arm as it is, and names the new parity suite beside `text-operator-case-exactness.test.ts`. ## Measured before the fix: the face table Executed, not read, at `980bc05e5b`. That is `main` after PR #20046 landed, and it moved nothing in `text-match-sql.ts` since the dispatch base `2274894cc4`. It ran on better-sqlite3 (SQLite 3.53.4) and sql.js (3.49.1), each through its driver's own `execute()`. Every cell was compared with `@objectstack/formula`'s `matchesFilterCondition`. The grid: stored values with U+0000 at the start, middle and end, without it, `''` and NULL; comparands with and without U+0000; each leaf bare and under `$not`. All five faces answered alike, and so did both engines: the direct read scope, the read scope through `NativeSQLStrategy` and through the echo, the native `where`, and the echo `where`. | operator | comparand without U+0000 | comparand holding U+0000 | |---|---|---| | `$contains` | narrows → **matches JS** | widens → **matches JS** | | `$notContains` | widens → **matches JS** | narrows → **matches JS** | | `$startsWith` | matches JS (unchanged, still `GLOB`) | widens → **matches JS** | | `$endsWith` | widens and narrows → **matches JS** | widens → **matches JS** | | `$icontains` | narrows → **matches JS** | widens → **matches JS** | | each of the above under `$not` | the opposite direction → **matches JS** | the opposite direction → **matches JS** | - **In this PR's pin grid** (`text-match-sqlite-nul.test.ts`, 128 cells), every face on both engines answered 88 cells differently from JS at base, and answers 0 at head. - **The ObjectQL execute face** goes through the engine to `driver-sql`, which is already fixed on `main`. It is the control, and it answered all 128 like JS at base and at head. - **The `unknown` arm on SQLite** emits `LIKE`, and SQLite's `LIKE` cuts at U+0000 too. That was measured on both engines: `'a'` + U+0000 + `'b'` is not `LIKE '%b%'`, and `'plain'` is `LIKE '%'` + U+0000 + `'%'`. The arm is untouched. It is the residue for dialects nothing answered for, PostgreSQL-like ones among them, and PostgreSQL has no `instr()`. Choosing a construct there would be a dialect guess. It is reported to the seat as a finding and noted in the header. - **`$like` / `$ilike`** are refused on every analytics face, before this arm: `READ_SCOPE_COMPILE_FAILED` on the read scope, `INVALID_FILTER` / 400 on the `where`. This package compiles no raw-pattern operator, so there is nothing to swap. ## Controllability (triage's point 1) - **Comparand side: no caller-controlled producer measured.** The RLS compile path (`RLSCompiler.compileFilter` → `compileCelToFilter`) resolves a string method's argument from one of two places: - a literal in the admin-authored policy; - a string attribute of `current_user` (id, organization id, email). The ids are system-generated. The email is refused at every auth entry measured when it holds U+0000, and it is read-only on the user object. Membership sets are arrays, which the string methods refuse. `plugin-sharing` emits id / owner membership scopes, not text operators. Sharing-rule criteria are admin-authored and evaluated through `engine.find`, the driver face that is already fixed. - **Stored-value side: a caller-controlled producer exists.** It is any persona that holds create or edit on an object with a text field. The engine's write path stores U+0000 unchanged under a non-system context (executed). Reaching a read scope also needs a deployment-authored `using` predicate with a string method over such a field. No shipped example authors one. ## Tests (at `d000746b1d`) - **New `src/__tests__/text-match-sqlite-nul.test.ts`, 29 tests.** It covers: - the 128-cell grid on every face, on better-sqlite3 and sql.js, each through its driver's `execute()`. That is the transport `plugin.ts` uses, and a bare sql.js bind would cut a comparand holding U+0000 before any construct saw it; - each cell equal to the JS answer AND to `driver-sql`'s own rows on the same engine. That is the parity pin that keeps the two construct tables from drifting; - literal pins for the headline cells; - the compiled constructs per shape, including `$startsWith` without U+0000 emitting `GLOB` and its escaped pattern byte for byte on all three compilers. It was committed first, red at base (`ced01cbad7`): 14 failed and 15 passed; each face on each engine had 88 of 128 cells differ; `driver-sql` had 0. - **Respelled pins, never loosened.** Four existing files asserted `GLOB` for shapes that no longer compile to it: `text-operator-case-exactness.test.ts`, `icontains-dialect-sql.test.ts`, `sql-dialect-vocabulary.test.ts` and `text-operator-non-text-column.test.ts`. - Each now names the exact per-operator construct. - The read-scope and echo loops check a per-operator regex plus "not ` LIKE `", where they used one `/GLOB/`. - The `$endsWith` / `$startsWith` compiled-text rows gained SQL assertions they did not have. - `pnpm --filter @objectstack/service-analytics test`: 121 files, 2718 tests passed. `typecheck` (`tsc --noEmit`, whose program lists the touched test files) exit 0. ### Ablations: each negative pin, one shape at a time, through `scripts/ablation-replace.mjs` The subject is `src/text-match-sql.ts`, imported relatively by every face, so vitest reads source and no `dist/` is involved. Every leg landed on disk (the anchor count went from 1 to 0 and the blob changed) and was restored with its blob equal to HEAD (`e913134711cb`) and `git diff HEAD` empty. | leg | mutation | red | |---|---|---| | A1 | `contains` back on `GLOB` | 21 tests; 60/128 cells per face and engine | | A2 | `ends` back on `GLOB` | 15 tests; 22/128 cells | | A3 | `starts` holding U+0000 back on `GLOB` | 11 tests; 6/128 cells | | A4 | `starts` without U+0000 moved off `GLOB` | 4 compiled-text tests (the byte-for-byte pins); rows unchanged, as expected | | A5 | empty `ends` sent to the suffix construct | 11 tests; 2/128 cells (`''` bare and under `$not`) | | A6 | `coalesce()` removed from the suffix | 14 tests; 12/128 cells (`''` under `$not`) | The first A6 attempt was a no-op. Its replacement text already occurred inside its own anchor, so the tool refused the leg and ran nothing. It was re-run with a non-overlapping anchor (the row above). `EXPLAIN QUERY PLAN` over an indexed TEXT column, on both engines: `$contains` / `$endsWith` scanned under `GLOB` and still scan, and `$startsWith` without U+0000 keeps its `SEARCH … USING INDEX`. ## Gates (at `d000746b1d`) - **The dispatch-gates derivation**: `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands`, run from the real diff. It gave 60 families, identical to the dispatch-time list. - 58 exit 0. - 2 are NOT MEASURED: `check:dual-build-cjs-loads` and `check:type-check-debt` both exited 3 with PREREQUISITE NOT MET, because they read the whole repo's built closure and this worktree built only the package's own. CI builds it first. - `--ran` reconciliation: 60 derived, 58 run, 2 NOT MEASURED, 0 unrun. - `GITHUB_TOKEN=… node scripts/check-issue-citations.mjs`: every citation this change adds resolves. - **Lint, a declared narrowing.** ESLint ran on the 6 changed TS files (`--no-inline-config --format json`): 6 files, 0 errors, 0 warnings. - The population is the config's `packages/**/*.{ts,…}` block; none of the six is in `NEVER_LINTED`. - Type-aware linting is not enabled: `--print-config` shows `parserOptions` without `project`. So this diff cannot move a verdict on an untouched file. - The full `pnpm lint` is CI's. ## Acceptance notes - **Surface.** The claim named `text-operator-case-exactness.test.ts` and new test files. Three more existing test files carried `GLOB` pins for the moved shapes and are respelled here (listed above). No call site, refusal door, driver, spec or formula file is touched. - **Stale prose, no assertion reads it.** These comments still say the SQLite arm is `GLOB` for every shape: `like-pattern.ts` (the `$icontains` note), `read-scope-sql.ts` (the `textMatch` and `$icontains` docblocks), `strategies/native-sql-strategy.ts`, `strategies/objectql-strategy.ts` and `strategies/types.ts`. They ride the next PR that touches those files. Carrier: none. - **Observation: `$icontains: ''`.** It is not a U+0000 cell, and this change does not move it. - These compilers, and `AnalyticsService.query` in-process, answer every non-NULL row for it, as they did under `GLOB '**'`. - The spec's `FILTER_TEXT_CASES` declares it refused (`INVALID_FILTER`), and `driver-sql` refuses it. - Reachability through the REST door is not measured. It is reported to the seat with its evidence. - `main` moved during this run only by `3557f85fa5` (a `driver-turso` README), which touches nothing here. No merge was needed. --- _Generated by [Claude Code](https://claude.ai/code/session_01Evb5jFDZGKQE9KG4jbMfMF)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1872e52 commit bf37b99

7 files changed

Lines changed: 568 additions & 52 deletions
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
fix(service-analytics): on SQLite, the text operators in analytics filters and read scopes compare the whole stored value and the whole comparand, instead of stopping at their first U+0000 (#20025)
6+
7+
Clause-②: no
8+
9+
`service-analytics` compiles its own SQL for `$contains`, `$notContains`, `$startsWith`, `$endsWith` and `$icontains`, in three places: the read scope applied to an analytics query (`compileScopedFilterToSql`), the `where` that `NativeSQLStrategy` executes, and the `ObjectQLStrategy` statement the `/analytics/sql` caller runs. On a SQLite datasource all three compiled these operators to `GLOB`, and SQLite's `glob()` reads both the pattern and the stored value only up to their first U+0000. Nothing raised, and the filter answered a different question. Measured on better-sqlite3 (SQLite 3.53.4) and sql.js (3.49.1), every one of these faces alike:
10+
11+
- a comparand holding U+0000 was cut at it, so `$contains` / `$endsWith` could match every row and `$notContains` none;
12+
- a stored value holding U+0000 was read only up to it, so `$contains` / `$endsWith` / `$icontains` missed a match after it, `$endsWith` could match what came before it, and `$notContains` / `$not` returned a row whose value does contain the comparand.
13+
14+
On a read scope the first kind widens what the scope admits and the second narrows or widens it. `driver-sql` and `driver-turso` already compile these operators this way (the #19999 and #20024 fixes); this package re-emits their construct table rather than importing it, and its copy had kept `GLOB`.
15+
16+
What changes: on the `sqlite` dialect these operators now compile to `driver-sql`'s constructs, cell for cell. `$contains`, `$notContains` and `$icontains` use `instr()`; `$endsWith` compares the value's trailing bytes over BLOB, with an empty comparand using `instr()` so it still matches every non-NULL value; a `$startsWith` comparand holding U+0000 uses `instr(…) = 1`. Such a filter now returns the rows `@objectstack/formula` and `driver-sql` return for it, on all three faces, bare and under `$not`, with `''` and NULL values included. The comparand is bound as written, so `*`, `?` and `[` in it are literal, as they were. `$icontains` still folds ASCII letters only, and `$notContains` still returns a row whose value is NULL.
17+
18+
What does not change:
19+
20+
- `$startsWith` with a comparand without U+0000 compiles to the same `GLOB` with the same bound pattern as before; the stored value's cut cannot change its answer. It keeps its index search (`EXPLAIN QUERY PLAN` over an indexed TEXT column on both engines); the other operators scanned the table under `GLOB` and still do.
21+
- The Postgres and MySQL arms, and every comparand refusal that runs before the text arm, are untouched.
22+
- A host that answers no SQL dialect for a SQLite datasource still gets the dialect-neutral `LIKE`, which SQLite also reads only up to the first U+0000.
23+
- `$like` and `$ilike` are still refused by these compilers, as before.

‎packages/services/service-analytics/src/__tests__/icontains-dialect-sql.test.ts‎

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@
2424
* ## Why the pins are per dialect, and what each cell's evidence is
2525
*
2626
* - **sqlite → EXECUTED.** Every assertion below that names row ids ran on
27-
* sql.js. The construct is `lower(col) GLOB lower(?)`, and SQLite's
28-
* `lower()` is ASCII-only — measured, `lower('CAFÉ')` is `cafÉ` — which is
27+
* sql.js. The construct is `instr(lower(col), lower(?)) > 0` ([#20025];
28+
* it was `lower(col) GLOB lower(?)` until the arm took `driver-sql`'s
29+
* U+0000-safe constructs), and SQLite's `lower()` is ASCII-only — measured, `lower('CAFÉ')` is `cafÉ` — which is
2930
* exactly the #4706 Q1 = A boundary this operator is ruled to.
3031
* - **postgres → byte-identical text.** `translate()` is correct there today,
3132
* so the correct diff is no diff: the emitted SQL and params are asserted
@@ -246,20 +247,24 @@ describe('[#15780] the compiled TEXT, per dialect — all three compilers', () =
246247
});
247248
});
248249

249-
it('sqlite compiles lower() over GLOB — one bound value, no ESCAPE clause', async () => {
250+
it('sqlite compiles lower() on both sides of instr() — one bound value, no ESCAPE clause', async () => {
251+
// [#20025] `instr()` rather than `GLOB`, because `glob()` cuts the pattern
252+
// and the stored value at their first U+0000 (`driver-sql`'s table since
253+
// #19999 / #20024). The fold is the same `lower()` on both sides, and the
254+
// comparand binds raw: `instr()` has no pattern language to escape.
250255
const out = await nativeSql({ name: { $icontains: 'acme' } }, 'sqlite');
251-
expect(out.sql).toContain('WHERE lower(name) GLOB lower($1)');
256+
expect(out.sql).toContain('WHERE instr(lower(name), lower($1)) > 0');
252257
expect(out.sql).not.toMatch(/translate\(/);
253-
expect(out.sql).not.toMatch(/ESCAPE/);
254-
expect(out.params).toEqual(['*acme*']);
258+
expect(out.sql).not.toMatch(/ESCAPE| GLOB /);
259+
expect(out.params).toEqual(['acme']);
255260

256261
const echo = await echoSql({ name: { $icontains: 'acme' } }, 'sqlite');
257-
expect(echo.sql).toContain('lower(name) GLOB lower($1)');
262+
expect(echo.sql).toContain('instr(lower(name), lower($1)) > 0');
258263
expect(echo.sql).not.toMatch(/translate\(/);
259-
expect(echo.params).toEqual(['*acme*']);
264+
expect(echo.params).toEqual(['acme']);
260265

261266
expect(compileScopedFilterToSql({ name: { $icontains: 'acme' } } as FilterCondition, 't', { dialect: 'sqlite' }))
262-
.toEqual({ sql: 'lower("t"."name") GLOB lower(?)', params: ['*acme*'] });
267+
.toEqual({ sql: 'instr(lower("t"."name"), lower(?)) > 0', params: ['acme'] });
263268
});
264269

265270
it('mysql compiles the nested-REPLACE binary fold — TEXT ONLY, NOT MEASURED on a server', async () => {
@@ -290,10 +295,12 @@ describe('[#15780] the compiled TEXT, per dialect — all three compilers', () =
290295
// The mirror of the control #15684 wrote for `$icontains`: the two families
291296
// must not collapse onto one path. If the fold ever leaks into these rows,
292297
// `$contains` gets back the case-insensitivity #4706 Q2 = A took away.
298+
// [#20025] `instr()` on SQLite since the arm left `GLOB` for `contains`;
299+
// what this control guards — no fold on the case-EXACT row — is unchanged.
293300
const sqlite = await nativeSql({ name: { $contains: 'acme' } }, 'sqlite');
294-
expect(sqlite.sql).toContain('WHERE name GLOB $1');
301+
expect(sqlite.sql).toContain('WHERE instr(name, $1) > 0');
295302
expect(sqlite.sql).not.toMatch(/lower\(/);
296-
expect(sqlite.params).toEqual(['*acme*']);
303+
expect(sqlite.params).toEqual(['acme']);
297304
const pg = await nativeSql({ name: { $contains: 'acme' } }, 'postgres');
298305
expect(pg.sql).toContain('WHERE name LIKE $1 ESCAPE $2');
299306
expect(pg.sql).not.toMatch(/translate\(/);
@@ -455,8 +462,9 @@ describe('[#15780] the three compilers, EXECUTED on a real SQLite engine', () =>
455462
getReadScope: (object: string) => (object === 'rows' ? (c.filter as FilterCondition) : null),
456463
} as DatasetScopedStrategyContext;
457464
const { sql, params } = await new NativeSQLStrategy().generateSql(query(undefined), scoped);
458-
expect(sql, c.name).toMatch(/GLOB/);
459-
expect(sql, c.name).not.toMatch(/translate\(/);
465+
// [#20025] The SQLite fold construct, named exactly rather than by `/GLOB/`.
466+
expect(sql, c.name).toMatch(/instr\(lower\([^)]*\), lower\(\$\d+\)\) > 0/);
467+
expect(sql, c.name).not.toMatch(/translate\(| LIKE /);
460468
expect(run(sql, params), c.name).toEqual([...c.expected]);
461469
}
462470
});
@@ -465,8 +473,8 @@ describe('[#15780] the three compilers, EXECUTED on a real SQLite engine', () =>
465473
for (const c of NAME_ICONTAINS) {
466474
const echo = await new ObjectQLStrategy().generateSql(query(c.filter), sqliteCtx);
467475
const native = await new NativeSQLStrategy().generateSql(query(c.filter), sqliteCtx);
468-
expect(echo.sql, c.name).toMatch(/GLOB/);
469-
expect(echo.sql, c.name).not.toMatch(/translate\(/);
476+
expect(echo.sql, c.name).toMatch(/instr\(lower\([^)]*\), lower\(\$\d+\)\) > 0/);
477+
expect(echo.sql, c.name).not.toMatch(/translate\(| LIKE /);
470478
expect(echo.params, c.name).toEqual(native.params);
471479
expect(run(echo.sql, echo.params), `echo of ${c.name}`).toEqual([...c.expected]);
472480
}

‎packages/services/service-analytics/src/__tests__/sql-dialect-vocabulary.test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,9 @@ describe('[#16206] the `unknown` arm\'s ROWS for a `sqlite3`-answering host, on
417417
const viaSqlite3 = await sqlite3Host.generateSql(query({ name: { $contains: 'acme' } }));
418418
const viaSqlite = await sqliteHost.generateSql(query({ name: { $contains: 'acme' } }));
419419
expect(viaSqlite3.sql).toContain('LIKE');
420-
expect(viaSqlite.sql).toContain('GLOB');
420+
// [#20025] `instr()` for `contains` since the `sqlite` arm took
421+
// `driver-sql`'s U+0000-safe constructs — case-exact, and never `LIKE`.
422+
expect(viaSqlite.sql).toContain('instr(name, $1) > 0');
423+
expect(viaSqlite.sql).not.toContain('LIKE');
421424
});
422425
});

0 commit comments

Comments
 (0)