Skip to content

Commit fd014b1

Browse files
os-warrenclaude
andauthored
fix(service-analytics): the unknown dialect arm folds $icontains with a portable construct, not translate() (#16204)
* fix(service-analytics): the `unknown` dialect arm folds `$icontains` with a portable construct, not `translate()` `normalizeSqlDialect` routes EVERYTHING it cannot name onto `unknown` — an unset hook, `'oracle'`, `'libsql'`, a `SqlDriver` given a class client. #15780 left that arm folding with `translate()` on the reading that it "was never broken", which held only for the dialects the arm was pictured as (mssql, oracle). SQLite reaches it through four embedder compositions, and SQLite has no `translate()`, so all three of this package's compilers emitted a statement the engine refuses — a documented operator answering 500 because one OPTIONAL field was left out. The arm now folds with one nested `REPLACE` per ASCII letter: the same chain the MySQL arm already used, minus its `CAST(… AS BINARY)`, so there is one builder and the two cannot fold different alphabets. It parses on every SQL dialect and is ASCII-only BY CONSTRUCTION, so it serves both families `unknown` conflates — PostgreSQL/Oracle-like keep `translate()`'s exact result set, SQLite-like get an answer at all. - `postgres` is untouched and byte-identical; so is the case-EXACT family on every arm (`fold` false makes the fold function the identity on both). - ⛔ Not `LOWER()`, which `driver-sql`'s own `unknown` arm uses: it follows the collation and would trade this parse failure for silently wrong rows on PostgreSQL, the Unicode fold #4706 Q1 = A rules out. - The pins that moved are re-aimed at the property, not regenerated, and the pre-fix bytes are kept as a control that the engine still refuses them. - `SqliteWasmDriver`'s `isSqlite` override — 0 direct test hits, the sole reason no in-repo SQLite driver lands on this arm — gets a direct pin, with a control showing the base class answers `unknown` for that very config. Refs #16028 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y * docs(changeset): the `unknown` arm's portable `$icontains` fold, graded and with its moved cells enumerated Refs #16028 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0e16fc4 commit fd014b1

10 files changed

Lines changed: 430 additions & 54 deletions
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.
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#16028] The `isSqlite` override, pinned DIRECTLY — the one thing that makes
5+
* this transport answer `"sqlite"` when something outside the driver asks which
6+
* SQL it speaks.
7+
*
8+
* ## Why this file exists
9+
*
10+
* `SqlDriver.dialectName` is public for exactly one consumer:
11+
* `service-analytics` compiles its own statements (an analytics `where`, an
12+
* ADR-0021 D-C read scope, the `/analytics/sql` echo) and needs the same
13+
* per-dialect construct choices the driver makes. Answering `'sqlite'` is what
14+
* routes `$icontains` onto `lower(col) GLOB lower(?)`; answering `'unknown'`
15+
* routes it onto the residue arm instead.
16+
*
17+
* The base class derives that answer by STRING-MATCHING `config.client` against
18+
* {@link SqlDriver}'s emission sets — and this transport passes a knex Client
19+
* CLASS, which is no string at all. So the correct answer here is produced by
20+
* one three-line override and by nothing else.
21+
*
22+
* ⚠️ #16028 measured that override at **0 direct test hits**: the only cover was
23+
* an indirect row-set pin (#15684), which would keep passing if the override
24+
* moved, because the ROWS come out the same either way — the driver runs its own
25+
* SQL through its own SQLite. What changes silently is the answer handed to a
26+
* package that compiles SQL for a DIFFERENT engine. That is the gap this file
27+
* closes, and it is the reason the control below is not decoration: it shows the
28+
* base class answering `'unknown'` for this exact config, so the pin above is a
29+
* measurement of the override rather than of the class hierarchy.
30+
*/
31+
32+
import { describe, it, expect, afterEach } from 'vitest';
33+
import { mkdtempSync, rmSync } from 'node:fs';
34+
import { tmpdir } from 'node:os';
35+
import { join } from 'node:path';
36+
import { SqlDriver } from '@objectstack/driver-sql';
37+
38+
import { SqliteWasmDriver } from '../src/index.js';
39+
40+
/** Nothing here connects — but every knex instance built is still torn down. */
41+
const opened: Array<{ disconnect(): Promise<void> }> = [];
42+
const dirs: string[] = [];
43+
44+
afterEach(async () => {
45+
await Promise.all(opened.splice(0).map((d) => d.disconnect().catch(() => {})));
46+
for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true });
47+
});
48+
49+
const track = <T extends { disconnect(): Promise<void> }>(d: T): T => {
50+
opened.push(d);
51+
return d;
52+
};
53+
54+
describe('[#16028] SqliteWasmDriver names its dialect', () => {
55+
it('answers "sqlite" — the answer service-analytics compiles against', () => {
56+
// Read WITHOUT connecting, deliberately: `service-analytics` asks this
57+
// while BUILDING a statement, so an answer that needed a live pool would
58+
// arrive after the SQL it decides.
59+
expect(track(new SqliteWasmDriver({ filename: ':memory:' })).dialectName).toBe('sqlite');
60+
});
61+
62+
it('…on a file-backed database too, and with persistence on', () => {
63+
const dir = mkdtempSync(join(tmpdir(), 'wasm-dialect-'));
64+
dirs.push(dir);
65+
const file = join(dir, 'test.db');
66+
expect(track(new SqliteWasmDriver({ filename: file })).dialectName).toBe('sqlite');
67+
expect(track(new SqliteWasmDriver({ filename: file, persist: 'on-write' })).dialectName).toBe('sqlite');
68+
});
69+
70+
it('the client is a CLASS, so no string table could have answered it', () => {
71+
// The override's premise, asserted rather than assumed: if this ever became
72+
// a string knex spelling, the base class would answer on its own and the
73+
// override would be dead code rather than the load-bearing line it is.
74+
const client = (SqliteWasmDriver.toKnexConfig({ filename: ':memory:' }) as { client: unknown }).client;
75+
expect(typeof client).toBe('function');
76+
expect(typeof client).not.toBe('string');
77+
});
78+
79+
it('CONTROL: the base class answers "unknown" for this very config', () => {
80+
// Delete `isSqlite` from the subclass and this is what `service-analytics`
81+
// would be told — #16028's residue arm, which for `$icontains` emitted a
82+
// statement SQLite cannot parse at all until that card. This is what makes
83+
// the pin above a measurement of the OVERRIDE rather than of the hierarchy.
84+
const base = track(new SqlDriver(SqliteWasmDriver.toKnexConfig({ filename: ':memory:', pool: { min: 0, max: 1 } })));
85+
expect(base.dialectName).toBe('unknown');
86+
});
87+
});

0 commit comments

Comments
 (0)