driver-sql: the isSqlite/isPostgres/isMysql getters recognise fewer knex client spellings than knex accepts, so a valid `client: 'postgres'` silently loses every dialect-specific behaviour

[os-zhuang]

Summary

SqlDriver's three dialect getters decide which dialect's behaviour applies by
matching config.client against a hard-coded pair of literals each:

protected get isSqlite(): boolean { const c = (this.config as any).client; return c === 'sqlite3' || c === 'better-sqlite3'; }
protected get isPostgres(): boolean { const c = (this.config as any).client; return c === 'pg' || c === 'postgresql'; }
protected get isMysql(): boolean { const c = (this.config as any).client; return c === 'mysql' || c === 'mysql2'; }

knex accepts more spellings than that. From knex/lib/constants.js (knex 3.3.0,
the version this repo pins):

const CLIENT_ALIASES = Object.freeze({ pg: 'postgres', postgresql: 'postgres', sqlite: 'sqlite3' });
const SUPPORTED_CLIENTS = Object.freeze([
 'mariadb', 'mssql', 'mysql', 'mysql2', 'oracledb',
 'postgres', 'pgnative', 'redshift', 'sqlite3', 'cockroachdb', 'better-sqlite3',
].concat(Object.keys(CLIENT_ALIASES)));

So client: 'postgres' — knex's own canonical name for the dialect, of which
pg and postgresql are merely the aliases — makes isPostgres return
false. Same shape for client: 'sqlite' (isSqlite → false) and for
cockroachdb / redshift, which speak the pg wire protocol.

Why it is not cosmetic

isPostgres gates real behaviour, not just logging. The clearest case is
nowColumnDefault, where the Postgres branch exists precisely to avoid a
timezone defect:

if (type === 'date') {
 if (this.isMysql) return this.knex.raw('(cast(utc_timestamp() as date))');
 if (this.isPostgres) return this.knex.raw("(timezone('utc', now())::date)");
}
return this.knex.fn.now(); // ← CURRENT_TIMESTAMP

With client: 'postgres' the driver falls through to knex.fn.now(), i.e. a
bare CURRENT_TIMESTAMP default on a DATE column — which resolves the
calendar day in the server's timezone, the exact defect that branch was
added to remove ("measured: a UTC-12 server records YESTERDAY", per the method's
own docblock). Nothing fails; the column is simply created with the wrong
default, on a configuration knex considers valid.

The file already disagrees with itself

DIALECT_CONNECT_TIMEOUT in the same class covers the wider set for the pg
family — pg, postgres, postgresql, cockroachdb — so two tables in one
file answer "is this Postgres?" differently depending on which one is asked.
This was measured while fixing #11389: a session pin placed behind
DIALECT_CONNECT_TIMEOUT's early return silently skipped redshift, which that
table omits while POSTGRES_WIRE_CLIENTS (added there) includes it. That fix
introduced a third list rather than widening isPostgres, deliberately — the
getters are read from regions of sql-driver.ts held by other in-flight claims,
so widening them was out of scope for that card.

Suggested direction (not a decision)

One dialect-identity source per dialect family, derived from a single table of
client spellings, with the connect-timeout table and the wire-protocol set
reading from it rather than each carrying its own literals. Whether redshift
and cockroachdb should be treated as Postgres for SQL emission (as opposed
to wire parsing) is a genuine question and probably wants a decision rather than
a guess — Redshift in particular diverges on DDL.

Where measured

packages/drivers/driver-sql/src/sql-driver.ts — the three getters, and
DIALECT_CONNECT_TIMEOUT / nowColumnDefault in the same file. Filed while
working #11389; not fixed there.

[os-zhuang]

Triage (Routine seat, hourly round): → pm:queue + domain:engine, type Bug.

Rationale: lands in packages/drivers/driver-sql. A knex-valid client: 'postgres' silently losing every dialect-specific behaviour is declared≠enforced with a silent-degradation blast radius. Family serial note: part of this round's driver-sql group (#11535 #11541 #11550 #11565 + blocked #11431) — engine seat answers fold-or-serial under the five gates at dispatch.

---

Generated by Claude Code

[huangyiirene]

Claim: PM loop round 3
Session: session_01VK8rFDtg8eREaxBGX99Csn
Branch: claude/issue-11550-dialect-getter-client-spellings
Worktree: objectstack-issue-11550
Domain: domain:engine
File surface: packages/drivers/driver-sql/src/sql-driver.ts (the three dialect getters and the identity tables they should share) + its test files (stop on breach; explain in the report)
Container & model: S, mode:subagent, model: opus
Clause-②: no — the narrow half dispatched here makes the driver recognise knex's own canonical spelling for dialects it already claims to support. No accept set widens: postgres is knex's canonical name of which pg / postgresql are the registered aliases, and sqlite likewise for sqlite3. This is declared = enforced catching up.
Serial constraints cleared, re-enumerated at this claim, ⛔ not reused from an earlier snapshot: all 15 open PRs fetched and their diffs read — only #11336 touches packages/drivers/driver-sql (CHANGELOG.md + package.json). No open PR occupies sql-driver.ts. Positive control: 11 files for #11699.

Regions on origin/main @ 2a6122bd9d (14494 lines): yours is isSqlite 4196, isPostgres 4202, isMysql 4208, plus DIALECT_CONNECT_TIMEOUT 4476 and POSTGRES_WIRE_CLIENTS **4584. Siblings this round are disjoint — #11635 at mapAggregateFunc**12786** / lowering call **7741**; #11565 atinitObjects**8773** /explainUnkeyableTextColumn` 13099.

⚠️ You own the getters this round, and two siblings READ them. #11635 selects its PG branch through isPostgres; #11565 selects a MySQL-only check through isMysql. Widening recognition is additive so I do not expect either to flip — but ⛔ if you change what an existing spelling (pg, postgresql, sqlite3, better-sqlite3, mysql, mysql2) resolves to, stop and report: that would move a sibling's behaviour under it.

⛔ This card is SPLIT — dispatch is the narrow half only

Dispatched: teach the identity getters knex's canonical spellings, so client: 'postgres' and client: 'sqlite' stop silently losing every dialect-specific behaviour. The card's own evidence is that this is not cosmetic: with client: 'postgres', nowColumnDefault falls through to knex.fn.now() — a bare CURRENT_TIMESTAMP default on a DATE column, which resolves the calendar day in the server's timezone, the exact defect that branch was added to remove ("a UTC-12 server records YESTERDAY", per its own docblock). Nothing fails; the column is just created wrong on a config knex considers valid.

⛔ NOT dispatched, and ⛔ not yours to decide: whether redshift and cockroachdb should be treated as Postgres for SQL emission. The card itself flags this as "a genuine question [that] probably wants a decision rather than a guess — Redshift in particular diverges on DDL". It is a support-scope question — it decides which databases the platform claims to emit correct DDL for — and that is the maintainer's, not mine and not yours. Filed separately; this claim links it once filed.

So: ⛔ do not add redshift or cockroachdb to any getter, and ⛔ do not remove them from POSTGRES_WIRE_CLIENTS (wire-protocol parsing is a different question from emission, and #11389 put them there deliberately). If the single-table refactor below cannot be done without deciding their membership, report the fork and stop — ⛔ do not settle it by picking whichever list you were already holding.

Suggested direction (the card's, and it is a suggestion)

One dialect-identity source per family, derived from a single table of client spellings, with DIALECT_CONNECT_TIMEOUT and the wire-protocol set reading from it rather than each carrying its own literals. ⚠️ The card records that the file already disagrees with itself — DIALECT_CONNECT_TIMEOUT covers pg / postgres / postgresql / cockroachdb while the getters cover two spellings, and #11389 added a third list (POSTGRES_WIRE_CLIENTS) rather than widening the getters, deliberately, because those regions were held by other claims at the time. They are not held now — this round is the moment that convergence is cheap.

⚠️ But converging the tables is exactly where the redshift/cockroachdb question bites: DIALECT_CONNECT_TIMEOUT already includes cockroachdb and POSTGRES_WIRE_CLIENTS already includes redshift. A naive merge would silently grant both emission identity — which is the decision that is out of scope. Keep the identity for emission set narrow and explicit, and let the other tables extend it rather than the reverse, or report if that shape does not work.

Pins

The behaviour worth pinning is nowColumnDefault's Postgres branch being selected under client: 'postgres' — i.e. the defect the card measured, not merely that a getter returns true. A getter-returns-true assertion passes for a refactor that breaks the branch it feeds. ⚠️ Reverse-verify whatever you write.

---

Generated by Claude Code