objectstack-ai
diff --git a/‎packages/drivers/driver-sql/src/sql-driver-aggregation-conformance.test.ts‎
Lines changed: 160 additions & 12 deletions b/‎packages/drivers/driver-sql/src/sql-driver-aggregation-conformance.test.ts‎
Lines changed: 160 additions & 12 deletions
@@ -11,7 +11,7 @@
  * aggregate that answers differently on one of them is one driver giving one
  * query two numbers, and only a shared table run on both can see it.
  *
- * ## Why a real better-sqlite3 database and not a SQL-string assertion
+ * ## Why a real database and not a SQL-string assertion
  *
  * `count_distinct` is the first entry in the vocabulary whose lowering is not a
  * function name — `COUNT(DISTINCT x)` puts a keyword inside the argument list.
@@ -22,8 +22,65 @@
  * instrument choice `turso-remote-filter-logic-conformance.test.ts` makes, and
  * for the same reason.
  *
+ * ## [#11456] The DRIVER axis — this suite runs on every dialect, not one
+ *
+ * It used to construct `client: 'better-sqlite3'` as a literal, so the shared
+ * aggregate-vocabulary standard — which exists precisely so two faces of one
+ * platform cannot answer one aggregation two ways — was measured on exactly one
+ * dialect of a driver that speaks three. ADR-0053 D-A3 makes the matrix
+ * `driver {SQLite, Postgres at minimum}`; its pagination sibling already went
+ * through `live-dialect-matrix.testkit.ts`. It now does too, so `pg` and
+ * `mysql` cells execute whenever `OS_TEST_POSTGRES_URL` / `OS_TEST_MYSQL_URL`
+ * are provisioned, and are a NAMED skip (a red under
+ * `OS_EXPECT_LIVE_DIALECT_MATRIX=1`) when they are not.
+ *
+ * ⛔ **No case is dialect-gated.** All eighteen tests run on every available
+ * cell; the conversion narrows nothing. Two of them needed a per-dialect FACE
+ * rather than a per-dialect exemption, and the difference matters — a `skipIf`
+ * would have been the silent narrowing this card was filed about.
+ *
+ * Measured under full CI parity (`OS_EXPECT_LIVE_DIALECT_MATRIX=1`, both URLs
+ * provisioned, `TZ=America/New_York`): **55 passed, 0 skipped** — 18 per cell
+ * on all three, plus the axis guard. The suite it replaced ran 18, on one.
+ *
+ * ### The readings the faces are built from, measured 2026-08-25
+ *
+ * All three cells on live servers at CI parity — PostgreSQL 16.13
+ * (`timezone=Asia/Shanghai`), MySQL 8.0.46 (`@@global.time_zone='+08:00'`),
+ * process `TZ=America/New_York`. Measured, not ported:
+ *
+ * | reading                             | sqlite        | pg             | mysql         |
+ * | ----------------------------------- | ------------- | -------------- | ------------- |
+ * | `count` / `count_distinct` result   | `6` number    | `"6"` STRING   | `6` number    |
+ * | `sum`/`avg`/`min`/`max` over `score` | `210` number  | `210` number   | `210` number  |
+ * | identifier quoting in emitted SQL   | `` `stage` `` | `"stage"`      | `` `stage` `` |
+ * | field-less `count_distinct` refusal | `INVALID_QUERY`/400 | identical | identical   |
+ *
+ * ⚠️ Two things in that table are worth reading slowly, because the tempting
+ * summary — "the live servers answer differently" — is wrong in both.
+ *
+ * FIRST: the divergent cell is **pg alone**, not "the live cells". mysql2 hands
+ * `COUNT`'s `BIGINT` back as a JS number; node-pg hands the same `BIGINT` back
+ * as a string rather than lose precision. So this is a property of one CLIENT
+ * LIBRARY's type parsing, and a face switched on `cell.live` would have been
+ * built on a distinction that is not the one doing the work.
+ *
+ * SECOND: within pg it is not uniform either. `count` arrives as a string while
+ * `sum`/`avg`/`min`/`max` over this fixture's `score` arrive as numbers,
+ * because `type: 'number'` lowers to a float column and node-pg parses float8
+ * to a number. The wire type is a property of the RESULT type per aggregate,
+ * not of the dialect — which is why the coercion below is unconditional rather
+ * than switched on `cell.id`. A `cell.id === 'pg'` branch would be green today
+ * and wrong the first time an aggregand's column type changed.
+ *
+ * {@link actualFor}'s `Number(r.n)` was already the right shape and needed no
+ * change; it is now load-bearing rather than incidental, and that is why it is
+ * spelled out here. The two tests that did NOT go through it are the two that
+ * needed work — see their own notes.
+ *
  * ## Reverse verification — direction predicted BEFORE it was run
  *
+ * Measured on the SQLite cell (the only one that existed when they were taken).
  * Two reverts, because the two mistakes this file guards against fail in
  * different ways and only one of them needs a database to see.
  *
@@ -58,12 +115,57 @@
  *   non-distinct lowering has nothing to refuse. `count_distinct(score)` stayed
  *   green throughout, exactly as predicted, which is why the table carries both
  *   columns.
+ *
+ * ## [#11456] Ablation — that the NEW cells execute, and can fail
+ *
+ * A converted matrix that finds zero live cells reports OK, so the conversion's
+ * own claim needed its own measurement. Predicted BEFORE running: mutating the
+ * `count_distinct` lowering to drop its `distinct` — revert (B) above, whose
+ * SQLite direction is already recorded — reddens the `live postgres` cell on
+ * the SAME four cases, proving that cell runs the assertions rather than
+ * merely connecting.
+ *
+ * Measured on live PostgreSQL 16.13, the mutation confirmed on disk by anchored
+ * counts (`distinct: true` 1 → 0, `distinct: false` 0 → 1) before any result
+ * was read: **8 failed / 29 passed / 1 skipped**, four in EACH executing cell —
+ * exactly the predicted direction and no other movement.
+ *
+ * The `live postgres` four, verbatim:
+ *
+ *  - `count_distinct(stage)` ungrouped: `value: 4` where the case says `2`;
+ *  - `count_distinct(stage) grouped by region`: `west` 3 where the case says 2;
+ *  - the emitted-SQL case, on
+ *    `expected 'select count("stage") as "n" from "co…' to contain 'count(distinct "stage")'`;
+ *  - the field-less refusal, now RESOLVING (`expected undefined to be defined`)
+ *    — `count(*)` is valid, so a non-distinct lowering has nothing to refuse.
+ *
+ * ⚠️ The third one is the load-bearing observation for this conversion, and it
+ * is why the derived quoting above is not circular: the PG cell failed against
+ * `count(distinct "stage")` — pg's OWN quoting, derived from pg's OWN client —
+ * while the SQLite cell failed against the backtick spelling in the same run.
+ * One assertion, two dialect-correct expectations, both of them able to fail.
+ * `count_distinct(score)` stayed GREEN on both cells throughout, as predicted,
+ * which is what keeps the four above from being a suite that simply broke.
+ *
+ * The mutation was restored and the restore VERIFIED rather than trusted —
+ * `sql-driver.ts` re-read as byte-identical to `origin/main`, and the suite
+ * re-run green. That check earned its keep: the `trap … EXIT INT TERM` fired
+ * but its `git checkout` used a repo-relative path after the script had `cd`'d
+ * into the package, so git answered `error: pathspec … did not match any
+ * file(s)` and the trap's own success line printed anyway. A restore leg that
+ * reports success and does nothing leaves every later measurement running
+ * against mutated code.
  */
 
 import { describe, it, expect, beforeAll, afterAll } from 'vitest';
 import { AGGREGATION_CASES, AGGREGATION_ROWS } from '@objectstack/spec/data';
 import type { AggregationCase, QueryAST } from '@objectstack/spec/data';
 import { SqlDriver } from './index.js';
+import {
+  DIALECT_CELLS,
+  declareDialectCell,
+  type DialectCell,
+} from './live-dialect-matrix.testkit.js';
 
 const CONFORMANCE_OBJECT = {
   name: 'conformance_agg',
@@ -92,7 +194,8 @@ const astFor = (c: AggregationCase): QueryAST => ({
  * The rows a case must produce, in the table's own order: `group` ascending for
  * a grouped case, one `null`-grouped row otherwise. Numbers are compared as
  * numbers — SQLite hands `avg` back as a float and `count` as an integer, and
- * neither is the property under test.
+ * [#11456] node-pg hands `count` back as the STRING `"6"` because `COUNT` is
+ * `bigint`. None of those is the property under test; the value is.
  */
 const actualFor = (c: AggregationCase, rows: Array<Record<string, unknown>>) => {
   // [#6401] The group value is read from the column the case SAYS it lands in —
@@ -104,20 +207,23 @@ const actualFor = (c: AggregationCase, rows: Array<Record<string, unknown>>) =>
     .sort((x, y) => String(x.group).localeCompare(String(y.group)));
 };
 
-describe('[#6409] SqlDriver — aggregate vocabulary conformance', () => {
+function declareAggregateVocabularyConformance(cell: DialectCell): void {
+describe(`[#6409] SqlDriver — aggregate vocabulary conformance (${cell.label})`, () => {
   let driver: SqlDriver;
 
   beforeAll(async () => {
-    driver = new SqlDriver({
-      client: 'better-sqlite3',
-      connection: { filename: ':memory:' },
-      useNullAsDefault: true,
-    });
+    driver = new SqlDriver(cell.config());
+    // The live cells own a per-FILE schema that a killed run can leave behind
+    // (`live-dialect-matrix.globalsetup.ts`'s teardown is best-effort by
+    // design), and a surviving table would seed twelve rows into a fixture
+    // whose every case counts six.
+    await driver.execute(`drop table if exists ${CONFORMANCE_OBJECT.name}`).catch(() => {});
     await driver.initObjects([CONFORMANCE_OBJECT as any]);
     for (const row of AGGREGATION_ROWS) await driver.create(CONFORMANCE_OBJECT.name, { ...row });
   });
 
   afterAll(async () => {
+    await driver.execute(`drop table if exists ${CONFORMANCE_OBJECT.name}`).catch(() => {});
     await driver.disconnect();
   });
 
@@ -155,6 +261,16 @@ describe('[#6409] SqlDriver — aggregate vocabulary conformance', () => {
    * property values cannot show: that the column arrives as a bound IDENTIFIER
    * (`??`) rather than interpolated into the statement text, which is what
    * keeps a caller's field name out of the SQL when `distinct` is in play.
+   *
+   * [#11456] The quoting is DERIVED from the cell's own dialect rather than
+   * spelled per cell. Measured: better-sqlite3 emits `` `stage` `` and pg emits
+   * `"stage"`, so the old literal backticks were a SQLite fact this suite was
+   * asserting about every dialect. A per-cell table of quote characters would
+   * work too and was rejected: the quote character is not the property under
+   * test, and a hand-kept table would need an entry the day a fourth dialect
+   * arrives — this cannot go stale. What IS asserted stays exactly what it was:
+   * the keyword `distinct` unquoted and INSIDE the parentheses, and the column
+   * bound as an identifier rather than interpolated as a string literal.
    */
   it('count_distinct compiles to count(distinct "column"), the column bound as an identifier', async () => {
     const knex = (driver as any).knex;
@@ -169,9 +285,8 @@ describe('[#6409] SqlDriver — aggregate vocabulary conformance', () => {
       knex.removeListener('query', capture);
     }
     expect(statements).toHaveLength(1);
-    // better-sqlite3 quotes identifiers with backticks; the keyword is SYNTAX,
-    // so it must appear unquoted and INSIDE the parentheses.
-    expect(statements[0]).toContain('count(distinct `stage`)');
+    const quoted = knex.client.wrapIdentifier('stage', (x: string) => x);
+    expect(statements[0]).toContain(`count(distinct ${quoted})`);
     // ⛔ The field must not have been interpolated as a string literal.
     expect(statements[0]).not.toContain("'stage'");
   });
@@ -181,6 +296,10 @@ describe('[#6409] SqlDriver — aggregate vocabulary conformance', () => {
    * emitting it. ADR-0112: the assertion is `code` AND `status`, never a bare
    * `toThrow` — the un-fixed driver threw here too (a 501 from the aggregate
    * door), so "it threw" cannot tell the two behaviours apart.
+   *
+   * [#11456] Measured identical on live pg: this refusal is raised by the
+   * driver before any dialect is consulted, which is the answer this cell is
+   * here to confirm rather than assume.
    */
   it('refuses count_distinct with no field — INVALID_QUERY / 400', async () => {
     const ast = {
@@ -206,12 +325,41 @@ describe('[#6409] SqlDriver — aggregate vocabulary conformance', () => {
   /**
    * The control that keeps the refusal above from being satisfiable by refusing
    * the field-less spelling in general: `count` still means `COUNT(*)`.
+   *
+   * [#11456] It used to read `toEqual([{ n: 6 }])`, which carried three facts —
+   * one row, the alias `n` as its ONLY key, and the value 6 — and could not
+   * survive the live cell, because pg answers `{ n: "6" }`: `COUNT` is `bigint`
+   * and node-pg hands bigints back as strings rather than lose precision. All
+   * three facts are kept and spelled out separately instead. ⛔ Not relaxed to
+   * a bare `toHaveProperty`: the key set is the DRIVER's own projection (it
+   * emits an explicit `as "n"`, never `*`), so it is dialect-independent and
+   * dropping it would be the silent narrowing this conversion exists to avoid.
    */
   it('count with no field still means COUNT(*)', async () => {
     const ast = {
       object: CONFORMANCE_OBJECT.name,
       aggregations: [{ function: 'count', alias: 'n' }],
     } as QueryAST;
-    expect(await driver.aggregate(CONFORMANCE_OBJECT.name, ast)).toEqual([{ n: 6 }]);
+    const rows = (await driver.aggregate(CONFORMANCE_OBJECT.name, ast)) as Array<Record<string, unknown>>;
+    expect(rows).toHaveLength(1);
+    expect(Object.keys(rows[0]), 'the alias is the only projected column').toEqual(['n']);
+    expect(Number(rows[0].n), 'count(*) over the six-row fixture').toBe(6);
   });
 });
+}
+
+/**
+ * [#11456] A matrix that silently finds zero cells reports OK — so the axis is
+ * asserted to be real before it is iterated, the same guard the #11635 suite
+ * carries. Without it, "converted to the live matrix" is a claim this file
+ * could satisfy while running nothing at all.
+ */
+describe('[#11456] the dialect axis this suite runs', () => {
+  it('runs every dialect this driver speaks', () => {
+    expect(DIALECT_CELLS.map((c) => c.id)).toEqual(['sqlite', 'pg', 'mysql']);
+  });
+});
+
+for (const cell of DIALECT_CELLS) {
+  declareDialectCell(cell, 'aggregate vocabulary conformance', declareAggregateVocabularyConformance);
+}