Skip to content

service-analytics: normalizeSqlDialect accepts only the three canonical names, so a host answering knex's own 'sqlite3' / 'pg' / 'mysql2' on the public sqlDialect hook is read as unknown — #11550's shape, one layer up #16206

Description

@os-warren

Filed by the os-dev seat while working #16028 (PR #16204). ⛔ Filed bare — domain:*, type and priority are triage's; this seat does not produce them. ⛔ Not fixed in #16204, deliberately: the remedy is a contract question, not a code change this seat may pick (reasoning below).

The gap

packages/services/service-analytics/src/text-match-sql.ts (measured on bdc02182b, unchanged by PR #16204):

const KNOWN_DIALECTS = new Set<string>(['sqlite', 'postgres', 'mysql']);

export function normalizeSqlDialect(name: string | undefined | null): AnalyticsSqlDialect {
  return typeof name === 'string' && KNOWN_DIALECTS.has(name) ? (name as AnalyticsSqlDialect) : 'unknown';
}

That set is driver-sql's SqlDialectName vocabulary, and it is the right one when the answer comes from SqlDriver.dialectName. But the reader is sqlDialectFor, which calls a host-supplied hook typed as free text — AnalyticsServiceConfig.sqlDialect?: (object: string) => string | undefined (analytics-service.ts:606). A host that owns a SQLite datasource and answers the spelling its own stack uses — knex's canonical 'sqlite3', or 'better-sqlite3', both of which driver-sql itself lists in SQLITE_EMIT_CLIENTS — is silently read as unknown.

Nothing tells the host. sqlDialectFor is tiered "cannot answer, do not block" by design, so a wrong answer and no answer are the same answer, and the host that tried hardest to help gets the residue arm.

Why it is not a one-line widening, and why this seat did not do it

Adding the knex spellings to KNOWN_DIALECTS is the lenient-alias shape AGENTS.md rules out at the consumer: the defect would be that the hook's accepted vocabulary is undeclared, and the fix belongs where the vocabulary is declared, not in a Set the compiler happens to read. The real question is which of these the platform means:

  • A. The hook's contract is SqlDialectName — the three canonical names. Then the gap is that nothing says so and nothing tells a host it answered out of contract: the remedy is a declared vocabulary (type or schema) plus a diagnostic on an unrecognised non-empty answer, not a wider Set.
  • B. The hook's contract is "whatever your driver calls itself" — knex client spellings included. Then normalizeSqlDialect needs the mapping table driver-sql already owns, and the two must be held together by a test rather than by hand.
  • C. Leave it. The population is embedder compositions, and after PR fix(service-analytics): the unknown dialect arm folds $icontains with a portable construct, not translate() #16204 the unknown arm is portable rather than broken, so the cost is a slower predicate, not a wrong or unparseable one.

⇒ Which of the three is a policy choice about a public extension point, so it is triage's and the maintainer's, not a dev seat's.

Severity context, so triage is not guessing

⚠️ Materially reduced by PR #16204, which is why this is filed as a separate card rather than folded into it. Before that PR, this spelling gap sent $icontains down an arm that emitted translate() and failed to parse on SQLite — a 500. After it, the unknown arm is portable and answers correct rows, so what remains is:

⛔ Also NOT MEASURED, and inherited from #16028: whether any shipped application answers this hook at all.

Prior art, same shape one layer down

#11550"the isSqlite/isPostgres/isMysql getters recognise fewer knex client spellings than knex accepts, so a valid client: 'postgres' silently loses every dialect-specific behaviour" — is this exact failure inside driver-sql, and it was ruled a defect there. Whether the ruling travels up to a host-supplied hook is precisely the question above: the driver reads a config it defined, while this reads a string an embedder wrote.

Related

#16028 (the unknown arm's fold) · PR #16204 · #11550 (the driver-side sibling, closed) · #15684 (the case-exact family on this same arm) · #11756 (the ruling that leaves mariadb unrecognised on purpose — evidence that "unrecognised" is sometimes deliberate, and part of why option C is real)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions