From 3b3ebcf02d0d16e5d927dba3cc7bd21b82922761 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Sun, 4 Oct 2026 15:52:56 +0200 Subject: [PATCH 1/3] Add CH.sql templates inside expressions, and Db.sql join/raw/empty CH.sql(type)`...` is a typed expression, CH.sql`...` an untyped one and CH.sql.cond`...` a condition, usable anywhere the builder takes one: select, where, ON, an UPDATE's SET. Interpolated columns, expressions and nested templates render as SQL; params stay placeholders (bound on Postgres); a builder query becomes a subquery compiled with the outer query, its tenant scope counted; a plain value becomes the dialect's escaped literal. Arrays and objects have no literal without their SQL type, so they fail the compile and point at param.of. sql.ident quotes plain names, sql.raw splices text, sql.join renders and joins values. Db.sql gains join (one bound value per item), raw and empty. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 ++ design/gap-review.md | 2 +- docs/database.md | 11 +++ docs/extending.md | 40 ++++++++- docs/reference.md | 1 + node_modules | 1 + src/ch/index.ts | 3 + src/ch/sql-template.test.ts | 95 ++++++++++++++++++++ src/ch/sql-template.ts | 159 ++++++++++++++++++++++++++++++++++ src/database.ts | 2 +- src/database/database.test.ts | 31 +++++++ src/database/sql.ts | 31 ++++++- 12 files changed, 377 insertions(+), 4 deletions(-) create mode 120000 node_modules create mode 100644 src/ch/sql-template.test.ts create mode 100644 src/ch/sql-template.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a8dc16..1bb0ca3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +- Add `CH.sql`: SQL templates inside expressions and conditions. `CH.sql(type)\`…\`` is a typed + `Expr`, ``CH.sql`…` `` an untyped one, `CH.sql.cond` a `Condition`; with `sql.ident`, `sql.raw` + and `sql.join`. Interpolated columns and params render as SQL and placeholders, a builder + query as a subquery compiled with the outer one, and a plain value as an escaped literal. +- Add `Db.sql.join`, `Db.sql.raw` and `Db.sql.empty` to statement templates. - Add `isNull()`, `isNotNull()`, `between()` and `notBetween()` on every expression, and variadic `CH.and(...)` / `CH.or(...)` that skip `undefined` and write one flat group. - Add `distinct()` and `distinctOn(...aliases)` to queries, on both dialects. diff --git a/design/gap-review.md b/design/gap-review.md index c6ff5f2..8586e00 100644 --- a/design/gap-review.md +++ b/design/gap-review.md @@ -26,7 +26,7 @@ builder; **P1** commonly used; **P2** niche. | --- | --- | --- | | ~~UPDATE builder: SET values and expressions, WHERE, RETURNING~~ (built) | ~120 | M | | ~~DELETE builder: WHERE, RETURNING~~ (built) | ~79 | S | -| A typed, value-binding `sql` template usable inside expressions; `sql.join` / `raw` / `empty` on `Db.sql` | ~163 | M | +| ~~A typed `sql` template usable inside expressions; `sql.join` / `raw` / `empty` on `Db.sql`~~ (built: `CH.sql`; plain values are literals, params are bound) | ~163 | M | | Postgres column types: `timestamptz` as `Date`, `timestamp`, `date`, `interval`, `varchar(n)`, serial / identity | 226 timestamp columns | S | | ~~DISTINCT (and DISTINCT ON)~~ (built) | ~10 | S | | ~~`FOR UPDATE` / `FOR SHARE` / `SKIP LOCKED` / `NOWAIT`~~ (built) | 7 | S | diff --git a/docs/database.md b/docs/database.md index 7e6b42f..a5d3cfd 100644 --- a/docs/database.md +++ b/docs/database.md @@ -122,6 +122,17 @@ const claimed = yield* Db.query( ) // ReadonlyArray<{ org_id: string; family: string }> ``` +`sql.join(values, separator?)` binds one value per item (or splices a `sql` item), joined by +`sql\`, \`` unless you pass another separator; `sql.raw(text)` splices text you control; and +`sql.empty` writes nothing, for an optional part: + +```ts +Db.sql`SELECT * FROM t WHERE id IN (${Db.sql.join(ids)})${archived ? Db.sql` AND archived` : Db.sql.empty}` +``` + +For SQL inside a builder query rather than a whole statement, use +[`CH.sql`](./extending.md#chsql--sql-templates-inside-a-query). + `query` and `execute` also take a plain `{ sql, parameters }` for SQL you have as text. `FromSqlClientOptions`: diff --git a/docs/extending.md b/docs/extending.md index c85b667..2b922e9 100644 --- a/docs/extending.md +++ b/docs/extending.md @@ -152,9 +152,47 @@ literal for all three. _(Backed by `src/ch/literal.test.ts > param.of`.)_ +## `CH.sql` — SQL templates inside a query + +For SQL the builder has no syntax for — a cast, an operator, a Postgres function — write a +template. It is an expression (or, with `.cond`, a condition), so it goes anywhere the builder +takes one: a select, a `where`, a join's ON, an UPDATE's SET. + +```ts +CH.from(Keys) + .select(($) => ({ + txid: CH.sql(PG.text)`pg_current_xact_id()::xid::text`, + next: CH.sql(PG.int8)`${$.uses} + ${1}`, + })) + .where(($) => [CH.sql.cond`${$.meta} @> ${CH.param.string("filter")}::jsonb`]) +// SELECT pg_current_xact_id()::xid::text AS "txid", "keys"."uses" + 1 AS "next" … +// WHERE "keys"."meta" @> $1::jsonb +``` + +Each `${value}` renders as the rest of the builder renders it: + +| Value | Renders as | +| --- | --- | +| a column, expression, or another template | its SQL | +| a `param.*` | a placeholder: bound on Postgres, a literal on ClickHouse | +| a builder query | `(subquery)`, compiled with the outer query, its tenant scope counted | +| a string, number, boolean, `Date`, `DateTime.Utc`, `null` | the dialect's escaped literal | +| `CH.sql.ident(name)` | the name quoted by the dialect; plain names only, dotted for `schema.table` | +| `CH.sql.raw(text)` | the text as-is — never from input | +| `CH.sql.join(values, separator?)` | each value rendered, joined by `", "` or `separator` | + +An array or object has no literal the template could write without its SQL type, so it fails the +compile with a `QueryBuilderError`; pass it as `param.of(type, name)` instead. `CH.sql(type)` +declares the result type, which decodes the value when it is selected; a bare ``CH.sql`…` `` +has none and costs the query its row schema, as `untypedExpr` does. A template condition is not +evidence of tenant scope. + +_(Backed by `src/ch/sql-template.test.ts` and `src/database/database.test.ts`.)_ + ## Raw escape hatches -`rawExpr` and `rawCond` take a SQL string as-is. `rawExpr` still requires the column type its +`rawExpr` and `rawCond` take a SQL string as-is; prefer `CH.sql`, which renders values and +params instead of taking text. `rawExpr` still requires the column type its SQL produces, so the row it lands in can still be decoded: ```ts diff --git a/docs/reference.md b/docs/reference.md index 95bb0ce..bbc62c1 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -118,6 +118,7 @@ time; see [Params and compilation](./params-and-compilation.md#what-each-kind-ac | Export | Purpose | | ------------------------- | ---------------------------------------------------------- | | `lit(value)` | Literal `Expr` from a `string` or `number` | +| `sql(type)\`…\`` / `sql\`…\`` / `sql.cond\`…\`` | A template `Expr` (typed or untyped) or `Condition`; `sql.ident`, `sql.raw`, `sql.join`. See [Extending](./extending.md#chsql--sql-templates-inside-a-query). Types `SqlTag`, `SqlTemplateValue`, `SqlRaw`, `SqlIdent` | | `rawExpr(sql, type)` | Unescaped `Expr` from SQL text, with a declared type | | `untypedExpr(sql)` | Unescaped `Expr` with no type — costs the row schema | | `rawCond(sql)` | Unescaped `Condition` from SQL text | diff --git a/node_modules b/node_modules new file mode 120000 index 0000000..5471656 --- /dev/null +++ b/node_modules @@ -0,0 +1 @@ +/Users/makisuo/Documents/GitHub/effect-clickhouse/node_modules \ No newline at end of file diff --git a/src/ch/index.ts b/src/ch/index.ts index bd557be..8e042f0 100644 --- a/src/ch/index.ts +++ b/src/ch/index.ts @@ -76,6 +76,9 @@ export { dynamicColumn, } from "./expr" +// SQL templates inside expressions and conditions. +export { sql, type SqlIdent, type SqlRaw, type SqlTag, type SqlTemplateValue } from "./sql-template" + // Subquery conditions. These accept a `CHQuery` as well as raw SQL, so they // supersede the string-only `exists`/`inSubquery` still exported from `./expr` // for direct subpath importers. diff --git a/src/ch/sql-template.test.ts b/src/ch/sql-template.test.ts new file mode 100644 index 0000000..72b6781 --- /dev/null +++ b/src/ch/sql-template.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from "@effect/vitest" +import { Effect } from "effect" +import * as CH from "./index" +import * as PG from "../postgres" +import * as T from "./types" +import { QueryBuilderError } from "./errors" + +const Keys = CH.table("keys", { id: PG.uuid, org: PG.text, meta: PG.jsonb(), uses: PG.int8 }, { tenantColumn: "org" }) +const Events = CH.table("events", { OrgId: CH.string, Count: CH.uint64, Name: CH.string }, { tenantColumn: "OrgId" }) + +describe("CH.sql", () => { + it("renders columns, params and plain values per dialect, with a typed row schema", () => { + const q = CH.from(Keys) + .select(($) => ({ txid: CH.sql(PG.text)`pg_current_xact_id()::xid::text`, next: CH.sql(PG.int8)`${$.uses} + ${1}` })) + .where(($) => [ + $.org.eq(CH.param.string("org")), + CH.sql.cond`${$.meta} @> ${CH.param.string("filter")}::jsonb`, + CH.sql.cond`${$.id} <> ${"it's"}`, + ]) + const compiled = PG.compileUnsafe(q, { org: "o", filter: '{"a":1}' }) + expect(compiled.sql).toContain('pg_current_xact_id()::xid::text AS "txid"') + expect(compiled.sql).toContain('"keys"."uses" + 1 AS "next"') + expect(compiled.sql).toContain(`"keys"."meta" @> $2::jsonb`) + expect(compiled.sql).toContain(`"keys"."id" <> 'it''s'`) + expect(compiled.parameters).toEqual(["o", '{"a":1}']) + expect(compiled.rowSchemaSource).toBe("derived") + expect(compiled.tenantScope).toBe("single-tenant") + + const ch = CH.compileUnsafe( + CH.from(Events).select(($) => ({ n: CH.sql(T.uint64)`${$.Count} * ${2}` })).where(($) => [CH.sql.cond`${$.Name} = ${"it's"}`]), + ) + expect(ch.sql).toContain("events.Count * 2 AS n") + expect(ch.sql).toContain("WHERE events.Name = 'it\\'s'") + }) + + it("an untyped template costs the row schema and names the alias", () => { + const compiled = PG.compileUnsafe(CH.from(Keys).select(() => ({ now: CH.sql`now()` }))) + expect(compiled.rowSchemaSource).toBe("none") + expect(compiled.untypedColumns).toEqual(["now"]) + }) + + it("raw, ident and join, nested templates, and a subquery compiled with the outer query", () => { + const Other = CH.table("other", { org: PG.text, id: PG.uuid }, { tenantColumn: "org" }) + const compiled = PG.compileUnsafe( + CH.from(Keys) + .select("id") + .where(($) => [ + CH.sql.cond`${CH.sql.ident("keys.org")} IN (${CH.sql.join(["a", "b", CH.param.string("c")])})`, + CH.sql.cond`${$.id} IN ${CH.from(Other).select("id").where(($o) => [$o.org.eq(CH.param.string("c"))])}`, + CH.sql.cond`${CH.sql`length(${$.org})`} > ${CH.sql.raw("2")}`, + ]), + { c: "z" }, + ) + expect(compiled.sql).toContain(`"keys"."org" IN ('a', 'b', $1)`) + expect(compiled.sql).toMatch(/"keys"\."id" IN \(SELECT[\s\S]*"other"\."org" = \$1\)/) + expect(compiled.sql).toContain(`length("keys"."org") > 2`) + expect(compiled.parameters).toEqual(["z"]) + // The subquery reads `other` pinned to the same param, and the outer query is unpinned. + expect(compiled.tenantScope).toBe("cross-tenant") + }) + + it("a subquery in a template counts toward tenant scope", () => { + const Other = CH.table("other", { org: PG.text, id: PG.uuid }, { tenantColumn: "org" }) + const scope = (inner: CH.CHQuery) => + PG.compileUnsafe( + CH.from(Keys).select("id").where(($) => [$.org.eq(CH.param.string("org")), CH.sql.cond`${$.id} IN ${inner}`]), + { org: "o" }, + ).tenantScope + expect(scope(CH.from(Other).select("id"))).toBe("cross-tenant") + expect(scope(CH.from(Other).select("id").where(($) => [$.org.eq(CH.param.string("org"))]))).toBe("single-tenant") + }) + + it.effect("a value with no literal form, a bad ident, or a missing param fails the compile", () => + Effect.gen(function* () { + const fails = (cond: CH.Condition, params: Record = {}) => + Effect.flip(PG.compile(CH.from(Keys).select("id").where(() => [cond]), params)) + const array = yield* fails(CH.sql.cond`x = ANY(${["a"] as any})`) + expect(array).toBeInstanceOf(QueryBuilderError) + expect(array.message).toContain("typed param") + expect((yield* fails(CH.sql.cond`${CH.sql.ident("a; drop")} = 1`)).message).toContain("not a plain identifier") + expect((yield* fails(CH.sql.cond`x = ${CH.param.string("missing")}`)).code).toBe("UnresolvedParam") + }), + ) + + it("works in a write's SET and WHERE", () => { + const compiled = PG.compileUnsafe( + CH.update(Keys) + .set(($) => ({ meta: CH.sql(PG.jsonb())`${$.meta} || ${CH.param.string("patch")}::jsonb` })) + .where(($) => [CH.sql.cond`${$.id} = ${CH.param.string("id")}::uuid`]), + { patch: "{}", id: "k" }, + ) + expect(compiled.sql).toBe('UPDATE "keys" SET "meta" = "meta" || $1::jsonb\nWHERE "id" = $2::uuid') + expect(compiled.parameters).toEqual(["{}", "k"]) + }) +}) diff --git a/src/ch/sql-template.ts b/src/ch/sql-template.ts new file mode 100644 index 0000000..621a8b5 --- /dev/null +++ b/src/ch/sql-template.ts @@ -0,0 +1,159 @@ +// `CH.sql`: SQL the builder has no syntax for, written as a template inside an +// expression or condition. +// +// CH.sql(PG.text)`pg_current_xact_id()::xid::text` +// CH.sql(T.uint64)`${$.Count} + ${1}` +// CH.sql.cond`${$.meta} @> ${CH.param.string("filter")}::jsonb` +// +// Each `${value}` renders the way the rest of the builder renders it: an +// expression, column or param as its SQL (a param stays a placeholder, bound on +// Postgres), a builder query as a subquery compiled with the outer query, and a +// plain value as the dialect's escaped literal. A value with no literal form (an +// array or object, whose SQL type the template cannot know) fails the compile; +// pass it as a typed param instead. Nothing interpolated becomes SQL text except +// through `sql.raw`, which says so. +// +// Like `Db.sql` for whole statements, but this one is a builder expression: it +// renders at compile time, for the dialect being compiled for. + +import { QueryBuilderError } from "./errors" +import { type Condition, type Expr, isExprLike, makeCond, makeExpr, makeUntypedExpr, toFragment } from "./expr" +import { compileCHUnsafe } from "./compile" +import type { CHQuery } from "./query" +import { renderSubquery } from "./subquery-context" +import type { CHType } from "./types" +import { compile, lazy, quoteIdentPath, type SqlFragment } from "../sql/sql-fragment" + +const RawTag = "@maple-dev/effect-orm/SqlRaw" +const IdentTag = "@maple-dev/effect-orm/SqlIdent" + +/** SQL text spliced as-is. From `sql.raw`. */ +export interface SqlRaw { + readonly _tag: typeof RawTag + readonly sql: string +} + +/** A name quoted by the dialect. From `sql.ident`. */ +export interface SqlIdent { + readonly _tag: typeof IdentTag + readonly name: string +} + +/** Anything a `CH.sql` template interpolates. */ +export type SqlTemplateValue = + | Expr + | Condition + | CHQuery + | SqlRaw + | SqlIdent + | string + | number + | bigint + | boolean + | Date + | null + | { readonly _tag: "Utc" } + +const tagged = (value: unknown, tag: Tag): value is { readonly _tag: Tag } => + typeof value === "object" && value !== null && (value as { readonly _tag?: unknown })._tag === tag + +const isQuery = (value: unknown): value is CHQuery => + typeof value === "object" && + value !== null && + "_state" in value && + typeof (value as { readonly select?: unknown }).select === "function" && + !("_tag" in value) + +// Plain names only, as for `Db.sql.identifier`: letters, digits and `_`, dotted +// for `schema.table`. +const PLAIN_NAME = /^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)*$/ + +/** One interpolated value as SQL, in the dialect being compiled for. */ +const renderValue = (value: unknown): string => { + if (tagged(value, RawTag)) return (value as SqlRaw).sql + if (tagged(value, IdentTag)) { + const { name } = value as SqlIdent + if (!PLAIN_NAME.test(name)) { + throw new QueryBuilderError({ + code: "InvalidLiteral", + message: `sql.ident: ${JSON.stringify(name)} is not a plain identifier (letters, digits, _, dotted for schema.table)`, + }) + } + return quoteIdentPath(name) + } + if (isQuery(value)) { + return `(${renderSubquery(value, (query) => + typeof query === "string" ? query : compileCHUnsafe(query, {}, { skipFormat: true, deferParams: true }).sql, + )})` + } + if (value === null) return "NULL" + if (typeof value === "bigint") return String(value) + if (typeof value === "number" && !Number.isFinite(value)) { + throw new QueryBuilderError({ code: "InvalidLiteral", message: `sql\`\`: ${value} has no SQL literal` }) + } + if ( + isExprLike(value) || + typeof value === "string" || + typeof value === "number" || + typeof value === "boolean" || + value instanceof Date || + tagged(value, "Utc") + ) { + return compile(toFragment(value)) + } + throw new QueryBuilderError({ + code: "InvalidLiteral", + message: `sql\`\`: cannot write ${Array.isArray(value) ? "an array" : typeof value} as a literal without its SQL type; pass it as a typed param (param.of(type, name))`, + }) +} + +/** The template as one fragment, rendered when the enclosing query compiles. */ +const fragmentOf = (strings: ReadonlyArray, values: ReadonlyArray): SqlFragment => + lazy(() => strings.reduce((text, part, index) => (index === 0 ? part : text + renderValue(values[index - 1]) + part), "")) + +type Tag = (strings: TemplateStringsArray, ...values: ReadonlyArray) => A + +export interface SqlTag { + /** + * An expression of `type`: `CH.sql(PG.text)\`...\``. The type decodes the + * value when it is selected, so the query keeps its row schema. + */ + (type: CHType): Tag> + /** + * An expression with no declared type. Selecting one costs the query its row + * schema, as `untypedExpr` does; give a type where the value is selected. + */ + (strings: TemplateStringsArray, ...values: ReadonlyArray): Expr + /** A condition, for `where`, `having`, a join's ON, or `CH.and` / `CH.or`. */ + readonly cond: Tag + /** SQL text spliced as-is. Only for text under your control, never for input. */ + readonly raw: (sql: string) => SqlRaw + /** A table or column name, quoted by the dialect. Plain names only, dotted for `schema.table`. */ + readonly ident: (name: string) => SqlIdent + /** Values joined by `separator` (default `, `), each rendered as an interpolation is. */ + readonly join: (values: ReadonlyArray, separator?: string) => Expr +} + +const isTemplateStrings = (value: unknown): value is TemplateStringsArray => + Array.isArray(value) && Object.hasOwn(value, "raw") + +/** + * SQL the builder has no syntax for, inside an expression or condition. See the + * module comment for how each interpolated value renders. + */ +export const sql: SqlTag = Object.assign( + (first: CHType | TemplateStringsArray, ...values: ReadonlyArray): any => { + if (isTemplateStrings(first)) return makeUntypedExpr(fragmentOf(first, values)) + const type = first + return (strings: TemplateStringsArray, ...inner: ReadonlyArray) => + makeExpr(fragmentOf(strings, inner), type.schema) + }, + { + cond: (strings: TemplateStringsArray, ...values: ReadonlyArray): Condition => + makeCond(fragmentOf(strings, values)), + raw: (text: string): SqlRaw => ({ _tag: RawTag, sql: text }), + ident: (name: string): SqlIdent => ({ _tag: IdentTag, name }), + join: (values: ReadonlyArray, separator = ", "): Expr => + makeUntypedExpr(lazy(() => values.map(renderValue).join(separator))), + }, +) diff --git a/src/database.ts b/src/database.ts index 9d223eb..1ff13c0 100644 --- a/src/database.ts +++ b/src/database.ts @@ -37,5 +37,5 @@ export { TransactionUnsupported, type TransactionError, } from "./database/errors" -export { sql, type SqlIdentifier, type SqlTemplate } from "./database/sql" +export { sql, type SqlIdentifier, type SqlRawText, type SqlTemplate } from "./database/sql" export type { DialectTransactions, IsolationLevel, TransactionSettings } from "./ch/dialect" diff --git a/src/database/database.test.ts b/src/database/database.test.ts index 8f79c87..8ab9efb 100644 --- a/src/database/database.test.ts +++ b/src/database/database.test.ts @@ -570,6 +570,23 @@ layer(Live, { excludeTestServices: true })("Database on PGlite", (it) => { }), ) + it.effect("CH.sql filters jsonb with @> and a bound param, and reads a typed cast", () => + Effect.gen(function* () { + yield* Db.execute(Db.sql`CREATE TABLE docs2 (id int4 PRIMARY KEY, meta jsonb NOT NULL)`) + const Docs = CH.table("docs2", { id: PG.int4, meta: PG.jsonb() }) + yield* Db.run(CH.insertInto(Docs).values([{ id: 1, meta: { tier: "gold", n: 1 } }, { id: 2, meta: { tier: "free" } }])) + const rows = yield* Db.run( + CH.from(Docs) + .select(($) => ({ id: $.id, tier: CH.sql(PG.text)`${$.meta}->>'tier'`, xid: CH.sql(PG.text)`pg_current_xact_id()::xid::text` })) + .where(($) => [CH.sql.cond`${$.meta} @> ${CH.param.string("filter")}::jsonb`]), + { filter: JSON.stringify({ tier: "gold" }) }, + ) + expect(rows).toHaveLength(1) + expect(rows[0]).toMatchObject({ id: 1, tier: "gold" }) + expect(typeof rows[0]!.xid).toBe("string") + }), + ) + it.effect("an insert inside a failed transaction rolls back", () => Effect.gen(function* () { const table = yield* freshTable @@ -599,6 +616,20 @@ describe("sql templates per dialect", () => { }), ) + it.effect("join binds each value, raw splices, empty writes nothing", () => + Effect.gen(function* () { + const ids = [1, 2, 3] + const statement = Db.sql`SELECT * FROM t WHERE id IN (${Db.sql.join(ids)})${false ? Db.sql` AND x` : Db.sql.empty} ORDER BY ${Db.sql.raw("id DESC")}` + expect(yield* renderTemplate(statement, postgresDialect)).toEqual({ + sql: "SELECT * FROM t WHERE id IN ($1, $2, $3) ORDER BY id DESC", + parameters: [1, 2, 3], + }) + const or = Db.sql.join([Db.sql`a = ${1}`, Db.sql`b = ${"x"}`], Db.sql` OR `) + expect(yield* renderTemplate(Db.sql`WHERE ${or}`, clickhouseDialect)).toEqual({ sql: "WHERE a = 1 OR b = 'x'", parameters: [] }) + expect(yield* renderTemplate(Db.sql.join([]), postgresDialect)).toEqual({ sql: "", parameters: [] }) + }), + ) + it.effect("ClickHouse writes escaped literals", () => Effect.gen(function* () { expect(yield* renderTemplate(template, clickhouseDialect)).toEqual({ diff --git a/src/database/sql.ts b/src/database/sql.ts index eb53485..3c6cedf 100644 --- a/src/database/sql.ts +++ b/src/database/sql.ts @@ -12,6 +12,13 @@ import { DatabaseError } from "./errors" const SqlTemplateTag = "@maple-dev/effect-orm/SqlTemplate" const IdentifierTag = "@maple-dev/effect-orm/SqlIdentifier" +const RawTag = "@maple-dev/effect-orm/SqlTemplateRaw" + +/** SQL text spliced as-is. From `sql.raw`. */ +export interface SqlRawText { + readonly _tag: typeof RawTag + readonly sql: string +} /** A name written as an identifier, quoted by the dialect. From `sql.identifier`. */ export interface SqlIdentifier { @@ -38,15 +45,36 @@ export const sql: { (strings: TemplateStringsArray, ...values: ReadonlyArray): SqlTemplate /** A table or column name, quoted by the dialect: `sql\`SELECT * FROM ${sql.identifier(table)}\``. */ readonly identifier: (name: string) => SqlIdentifier + /** + * Values joined by `separator` (default `sql\`, \``): a plain value is bound, a + * `sql\`...\`` spliced. `sql\`id IN (${sql.join(ids)})\`` binds one value per id. + */ + readonly join: (values: ReadonlyArray, separator?: SqlTemplate) => SqlTemplate + /** SQL text spliced as-is. Only for text under your control, never for input. */ + readonly raw: (text: string) => SqlRawText + /** A template that writes nothing, for an optional part: `${cond ? sql\`AND x\` : sql.empty}`. */ + readonly empty: SqlTemplate } = Object.assign( (strings: TemplateStringsArray, ...values: ReadonlyArray): SqlTemplate => ({ _tag: SqlTemplateTag, strings: [...strings], values, }), - { identifier: (name: string): SqlIdentifier => ({ _tag: IdentifierTag, name }) }, + { + identifier: (name: string): SqlIdentifier => ({ _tag: IdentifierTag, name }), + join: (values: ReadonlyArray, separator?: SqlTemplate): SqlTemplate => { + const sep: SqlTemplate = separator ?? { _tag: SqlTemplateTag, strings: [", "], values: [] } + const interleaved = values.flatMap((value, index) => (index === 0 ? [value] : [sep, value])) + return { _tag: SqlTemplateTag, strings: Array.from({ length: interleaved.length + 1 }, () => ""), values: interleaved } + }, + raw: (text: string): SqlRawText => ({ _tag: RawTag, sql: text }), + empty: { _tag: SqlTemplateTag, strings: [""], values: [] } satisfies SqlTemplate, + }, ) +const isRaw = (value: unknown): value is SqlRawText => + typeof value === "object" && value !== null && "_tag" in value && value._tag === RawTag + const isIdentifier = (value: unknown): value is SqlIdentifier => typeof value === "object" && value !== null && "_tag" in value && value._tag === IdentifierTag @@ -84,6 +112,7 @@ export const renderTemplate = ( const value = current.values[index - 1] if (isSqlTemplate(value)) return text + render(value) + part if (isIdentifier(value)) return text + identifier(dialect, value.name) + part + if (isRaw(value)) return text + value.sql + part if (dialect.params._tag === "inline") return text + checkedLiteral(dialect, value, "a sql`` value") + part parameters.push(value) return text + dialect.params.placeholder(parameters.length, "") + part From 639e0586c863cdec32c32b40e3ba886aa0e5d2bc Mon Sep 17 00:00:00 2001 From: Makisuo Date: Sun, 4 Oct 2026 16:00:28 +0200 Subject: [PATCH 2/3] Parenthesize CH.sql templates and recognise raw/ident by identity From review: - A template condition was spliced bare, so CH.sql.cond`a OR b` in a where list became `a OR b AND org = $1`, reading every tenant while the query still reported single-tenant. Templates now render in parentheses (expressions too); sql.join stays bare to fit IN (...). - sql.raw / sql.ident, and Db.sql's templates, identifiers and raw text, were recognised by a string _tag, which an object parsed from request JSON could forge to splice SQL (pre-existing for Db.sql templates). They are now recognised by identity through a private WeakSet; a forged object is a value: bound on Postgres, an escaped literal or an error on ClickHouse. - A negative number after `-` wrote `10--1`, commenting out the rest of the line; negatives, and params ClickHouse inlines, are parenthesized. - An empty sql.join / Db.sql.join fails instead of writing `IN ()`; an invalid Date is a typed failure; DateTime is detected with DateTime.isDateTime; a unionAll gets a message pointing at fromUnion. - Stop tracking the node_modules symlink the worktree committed, and ignore node_modules whether it is a directory or a link. Co-Authored-By: Claude Opus 5.5 --- .gitignore | 2 +- docs/database.md | 4 +- docs/extending.md | 23 +++++++---- node_modules | 1 - src/ch/sql-template.test.ts | 70 ++++++++++++++++++++++++++------- src/ch/sql-template.ts | 73 ++++++++++++++++++++++++++++++----- src/database/database.test.ts | 20 +++++++++- src/database/sql.ts | 49 ++++++++++++++++------- 8 files changed, 193 insertions(+), 49 deletions(-) delete mode 120000 node_modules diff --git a/.gitignore b/.gitignore index 50670f2..96994e2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ -node_modules/ +node_modules dist/ coverage/ *.tgz diff --git a/docs/database.md b/docs/database.md index a5d3cfd..ff1321d 100644 --- a/docs/database.md +++ b/docs/database.md @@ -124,7 +124,9 @@ const claimed = yield* Db.query( `sql.join(values, separator?)` binds one value per item (or splices a `sql` item), joined by `sql\`, \`` unless you pass another separator; `sql.raw(text)` splices text you control; and -`sql.empty` writes nothing, for an optional part: +`sql.empty` writes nothing, for an optional part. A `join` of no values fails when the statement +renders, since `IN ()` is not SQL. Templates, identifiers and raw text are recognised by identity, +so an object parsed from request JSON is bound as a value, never spliced: ```ts Db.sql`SELECT * FROM t WHERE id IN (${Db.sql.join(ids)})${archived ? Db.sql` AND archived` : Db.sql.empty}` diff --git a/docs/extending.md b/docs/extending.md index 2b922e9..4abf0a3 100644 --- a/docs/extending.md +++ b/docs/extending.md @@ -165,8 +165,8 @@ CH.from(Keys) next: CH.sql(PG.int8)`${$.uses} + ${1}`, })) .where(($) => [CH.sql.cond`${$.meta} @> ${CH.param.string("filter")}::jsonb`]) -// SELECT pg_current_xact_id()::xid::text AS "txid", "keys"."uses" + 1 AS "next" … -// WHERE "keys"."meta" @> $1::jsonb +// SELECT (pg_current_xact_id()::xid::text) AS "txid", ("keys"."uses" + 1) AS "next" … +// WHERE ("keys"."meta" @> $1::jsonb) ``` Each `${value}` renders as the rest of the builder renders it: @@ -179,13 +179,22 @@ Each `${value}` renders as the rest of the builder renders it: | a string, number, boolean, `Date`, `DateTime.Utc`, `null` | the dialect's escaped literal | | `CH.sql.ident(name)` | the name quoted by the dialect; plain names only, dotted for `schema.table` | | `CH.sql.raw(text)` | the text as-is — never from input | -| `CH.sql.join(values, separator?)` | each value rendered, joined by `", "` or `separator` | - -An array or object has no literal the template could write without its SQL type, so it fails the -compile with a `QueryBuilderError`; pass it as `param.of(type, name)` instead. `CH.sql(type)` +| `CH.sql.join(values, separator?)` | each value rendered, joined by `", "` or `separator`; not parenthesized, so it fits `IN (${…})`; an empty list fails the compile | + +A template is written in parentheses, so `CH.sql.cond\`a OR b\`` in a `where` list stays one +operand instead of swallowing the conditions it is AND-joined with. A negative number (or a +param ClickHouse inlines as one) is parenthesized too, so `10-${n}` cannot become the comment +`10--1`. + +`sql.raw` and `sql.ident` values are recognised by identity, not by their fields, so an object +parsed from request JSON can never pass for one. An array or object has no literal the template +could write without its SQL type, so it fails the compile with a `QueryBuilderError`; pass it as +`param.of(type, name)` instead. A `unionAll` cannot be interpolated; select from it with +`fromUnion` and interpolate that. `CH.sql(type)` declares the result type, which decodes the value when it is selected; a bare ``CH.sql`…` `` has none and costs the query its row schema, as `untypedExpr` does. A template condition is not -evidence of tenant scope. +evidence of tenant scope; being parenthesized, it cannot cancel the evidence of the conditions +beside it either. _(Backed by `src/ch/sql-template.test.ts` and `src/database/database.test.ts`.)_ diff --git a/node_modules b/node_modules deleted file mode 120000 index 5471656..0000000 --- a/node_modules +++ /dev/null @@ -1 +0,0 @@ -/Users/makisuo/Documents/GitHub/effect-clickhouse/node_modules \ No newline at end of file diff --git a/src/ch/sql-template.test.ts b/src/ch/sql-template.test.ts index 72b6781..a51c4f0 100644 --- a/src/ch/sql-template.test.ts +++ b/src/ch/sql-template.test.ts @@ -9,7 +9,7 @@ const Keys = CH.table("keys", { id: PG.uuid, org: PG.text, meta: PG.jsonb(), use const Events = CH.table("events", { OrgId: CH.string, Count: CH.uint64, Name: CH.string }, { tenantColumn: "OrgId" }) describe("CH.sql", () => { - it("renders columns, params and plain values per dialect, with a typed row schema", () => { + it("renders columns, params and plain values per dialect, parenthesized, with a typed row schema", () => { const q = CH.from(Keys) .select(($) => ({ txid: CH.sql(PG.text)`pg_current_xact_id()::xid::text`, next: CH.sql(PG.int8)`${$.uses} + ${1}` })) .where(($) => [ @@ -18,10 +18,10 @@ describe("CH.sql", () => { CH.sql.cond`${$.id} <> ${"it's"}`, ]) const compiled = PG.compileUnsafe(q, { org: "o", filter: '{"a":1}' }) - expect(compiled.sql).toContain('pg_current_xact_id()::xid::text AS "txid"') - expect(compiled.sql).toContain('"keys"."uses" + 1 AS "next"') - expect(compiled.sql).toContain(`"keys"."meta" @> $2::jsonb`) - expect(compiled.sql).toContain(`"keys"."id" <> 'it''s'`) + expect(compiled.sql).toContain('(pg_current_xact_id()::xid::text) AS "txid"') + expect(compiled.sql).toContain('("keys"."uses" + 1) AS "next"') + expect(compiled.sql).toContain(`("keys"."meta" @> $2::jsonb)`) + expect(compiled.sql).toContain(`("keys"."id" <> 'it''s')`) expect(compiled.parameters).toEqual(["o", '{"a":1}']) expect(compiled.rowSchemaSource).toBe("derived") expect(compiled.tenantScope).toBe("single-tenant") @@ -29,8 +29,22 @@ describe("CH.sql", () => { const ch = CH.compileUnsafe( CH.from(Events).select(($) => ({ n: CH.sql(T.uint64)`${$.Count} * ${2}` })).where(($) => [CH.sql.cond`${$.Name} = ${"it's"}`]), ) - expect(ch.sql).toContain("events.Count * 2 AS n") - expect(ch.sql).toContain("WHERE events.Name = 'it\\'s'") + expect(ch.sql).toContain("(events.Count * 2) AS n") + expect(ch.sql).toContain("WHERE (events.Name = 'it\\'s')") + }) + + it("a template OR cannot swallow the conditions it is AND-joined with", () => { + const compiled = PG.compileUnsafe( + CH.from(Keys) + .select("id") + .where(($) => [CH.sql.cond`${$.uses} = 1 OR ${$.uses} = 2`, $.org.eq(CH.param.string("org"))]), + { org: "o" }, + ) + expect(compiled.sql).toMatch(/WHERE \("keys"\."uses" = 1 OR "keys"\."uses" = 2\)\s+AND "keys"\."org" = \$1/) + const anded = CH.compileUnsafe( + CH.from(Events).select("Name").where(($) => [CH.and(CH.sql.cond`${$.Count} = 1 OR ${$.Count} = 2`, $.OrgId.eq("o"))]), + ) + expect(anded.sql).toContain("((events.Count = 1 OR events.Count = 2) AND events.OrgId = 'o')") }) it("an untyped template costs the row schema and names the alias", () => { @@ -51,11 +65,10 @@ describe("CH.sql", () => { ]), { c: "z" }, ) - expect(compiled.sql).toContain(`"keys"."org" IN ('a', 'b', $1)`) - expect(compiled.sql).toMatch(/"keys"\."id" IN \(SELECT[\s\S]*"other"\."org" = \$1\)/) - expect(compiled.sql).toContain(`length("keys"."org") > 2`) + expect(compiled.sql).toContain(`("keys"."org" IN ('a', 'b', $1))`) + expect(compiled.sql).toMatch(/\("keys"\."id" IN \(SELECT[\s\S]*"other"\."org" = \$1\)\)/) + expect(compiled.sql).toContain(`((length("keys"."org")) > 2)`) expect(compiled.parameters).toEqual(["z"]) - // The subquery reads `other` pinned to the same param, and the outer query is unpinned. expect(compiled.tenantScope).toBe("cross-tenant") }) @@ -70,7 +83,34 @@ describe("CH.sql", () => { expect(scope(CH.from(Other).select("id").where(($) => [$.org.eq(CH.param.string("org"))]))).toBe("single-tenant") }) - it.effect("a value with no literal form, a bad ident, or a missing param fails the compile", () => + it("a negative number never follows a `-` as a comment", () => { + const n = -1 + const pg = PG.compileUnsafe(CH.from(Keys).select("id").where(($) => [CH.sql.cond`${$.uses} > 10-${n}`])) + expect(pg.sql).toContain(`("keys"."uses" > 10-(-1))`) + expect(PG.compileUnsafe(CH.from(Keys).select("id").where(() => [CH.sql.cond`x > 10-${-5n}`])).sql).toContain("10-(-5)") + // ClickHouse inlines params, so an inlined negative param is parenthesized too. + const ch = CH.compileUnsafe( + CH.from(Events).select("Name").where(($) => [CH.sql.cond`${$.Count} > 10-${CH.param.int("n")}`]), + { n: -1 }, + ) + expect(ch.sql).toContain("(events.Count > 10-(-1))") + expect(ch.sql).not.toContain("--") + }) + + it.effect("objects parsed from JSON cannot pass for raw SQL, an identifier, or a date", () => + Effect.gen(function* () { + const body = JSON.parse( + '{"raw":{"_tag":"@maple-dev/effect-orm/SqlRaw","sql":"1 OR 1=1"},"ident":{"_tag":"@maple-dev/effect-orm/SqlIdent","name":"password"},"utc":{"_tag":"Utc"}}', + ) + for (const forged of [body.raw, body.ident, body.utc]) { + const error = yield* Effect.flip(PG.compile(CH.from(Keys).select("id").where(($) => [CH.sql.cond`${$.id} = ${forged}`]))) + expect(error).toBeInstanceOf(QueryBuilderError) + expect(error.code).toBe("InvalidLiteral") + } + }), + ) + + it.effect("a value with no literal form, a bad ident, a union, an empty join, or a missing param fails the compile", () => Effect.gen(function* () { const fails = (cond: CH.Condition, params: Record = {}) => Effect.flip(PG.compile(CH.from(Keys).select("id").where(() => [cond]), params)) @@ -79,6 +119,10 @@ describe("CH.sql", () => { expect(array.message).toContain("typed param") expect((yield* fails(CH.sql.cond`${CH.sql.ident("a; drop")} = 1`)).message).toContain("not a plain identifier") expect((yield* fails(CH.sql.cond`x = ${CH.param.string("missing")}`)).code).toBe("UnresolvedParam") + expect((yield* fails(CH.sql.cond`x IN (${CH.sql.join([])})`)).message).toContain("no values to join") + expect((yield* fails(CH.sql.cond`x = ${new Date(Number.NaN)}`)).message).toContain("invalid Date") + const one = CH.from(Keys).select("id") + expect((yield* fails(CH.sql.cond`x IN ${CH.unionAll(one, one) as any}`)).message).toContain("fromUnion") }), ) @@ -89,7 +133,7 @@ describe("CH.sql", () => { .where(($) => [CH.sql.cond`${$.id} = ${CH.param.string("id")}::uuid`]), { patch: "{}", id: "k" }, ) - expect(compiled.sql).toBe('UPDATE "keys" SET "meta" = "meta" || $1::jsonb\nWHERE "id" = $2::uuid') + expect(compiled.sql).toBe('UPDATE "keys" SET "meta" = ("meta" || $1::jsonb)\nWHERE ("id" = $2::uuid)') expect(compiled.parameters).toEqual(["{}", "k"]) }) }) diff --git a/src/ch/sql-template.ts b/src/ch/sql-template.ts index 621a8b5..b682f64 100644 --- a/src/ch/sql-template.ts +++ b/src/ch/sql-template.ts @@ -15,7 +15,14 @@ // // Like `Db.sql` for whole statements, but this one is a builder expression: it // renders at compile time, for the dialect being compiled for. +// +// A template is parenthesized where it lands, so `CH.sql.cond\`a OR b\`` in a +// `where` list cannot swallow the conditions it is AND-joined with. `sql.raw` and +// `sql.ident` values are recognised by identity (a private WeakSet), never by a +// field, so an object parsed from request JSON cannot pass for one. +import { DateTime } from "effect" +import { currentDialect } from "./dialect" import { QueryBuilderError } from "./errors" import { type Condition, type Expr, isExprLike, makeCond, makeExpr, makeUntypedExpr, toFragment } from "./expr" import { compileCHUnsafe } from "./compile" @@ -52,10 +59,20 @@ export type SqlTemplateValue = | boolean | Date | null - | { readonly _tag: "Utc" } + | DateTime.DateTime +// The values `sql.raw` and `sql.ident` made. Identity, not a `_tag` check: a +// `_tag` survives JSON, so request input could forge raw SQL. +const minted = new WeakSet() +const mint = (value: A): A => { + minted.add(value) + return value +} const tagged = (value: unknown, tag: Tag): value is { readonly _tag: Tag } => - typeof value === "object" && value !== null && (value as { readonly _tag?: unknown })._tag === tag + typeof value === "object" && value !== null && minted.has(value) && (value as { readonly _tag?: unknown })._tag === tag + +const isUnion = (value: unknown): boolean => + typeof value === "object" && value !== null && (value as { readonly _tag?: unknown })._tag === "CHUnionQuery" && "_state" in value const isQuery = (value: unknown): value is CHQuery => typeof value === "object" && @@ -68,8 +85,16 @@ const isQuery = (value: unknown): value is CHQuery => // for `schema.table`. const PLAIN_NAME = /^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)*$/ +/** + * A rendered value that starts with `-` is parenthesized: `10-${-1}` would + * otherwise write `10--1`, and `--` comments out the rest of the line. + */ +const guarded = (sql: string): string => (sql.startsWith("-") ? `(${sql})` : sql) + /** One interpolated value as SQL, in the dialect being compiled for. */ -const renderValue = (value: unknown): string => { +const renderValue = (value: unknown): string => guarded(renderValueRaw(value)) + +const renderValueRaw = (value: unknown): string => { if (tagged(value, RawTag)) return (value as SqlRaw).sql if (tagged(value, IdentTag)) { const { name } = value as SqlIdent @@ -81,6 +106,12 @@ const renderValue = (value: unknown): string => { } return quoteIdentPath(name) } + if (isUnion(value)) { + throw new QueryBuilderError({ + code: "InvalidArguments", + message: "sql``: a unionAll cannot be interpolated; select from it with fromUnion(union, alias) and interpolate that query", + }) + } if (isQuery(value)) { return `(${renderSubquery(value, (query) => typeof query === "string" ? query : compileCHUnsafe(query, {}, { skipFormat: true, deferParams: true }).sql, @@ -91,13 +122,21 @@ const renderValue = (value: unknown): string => { if (typeof value === "number" && !Number.isFinite(value)) { throw new QueryBuilderError({ code: "InvalidLiteral", message: `sql\`\`: ${value} has no SQL literal` }) } + if (value instanceof Date && Number.isNaN(value.getTime())) { + throw new QueryBuilderError({ code: "InvalidLiteral", message: "sql``: an invalid Date has no SQL literal" }) + } + // A param inlined as a literal (ClickHouse) is only known after rendering, so + // it is parenthesized here: a negative value must not follow a `-`. + if (isExprLike(value) && "_paramName" in value && currentDialect().params._tag === "inline") { + return `(${compile(value.toFragment())})` + } if ( isExprLike(value) || typeof value === "string" || typeof value === "number" || typeof value === "boolean" || value instanceof Date || - tagged(value, "Utc") + DateTime.isDateTime(value) ) { return compile(toFragment(value)) } @@ -107,9 +146,12 @@ const renderValue = (value: unknown): string => { }) } -/** The template as one fragment, rendered when the enclosing query compiles. */ +/** + * The template as one fragment, rendered when the enclosing query compiles, + * in parentheses so it binds as one operand wherever it lands. + */ const fragmentOf = (strings: ReadonlyArray, values: ReadonlyArray): SqlFragment => - lazy(() => strings.reduce((text, part, index) => (index === 0 ? part : text + renderValue(values[index - 1]) + part), "")) + lazy(() => `(${strings.reduce((text, part, index) => (index === 0 ? part : text + renderValue(values[index - 1]) + part), "")})`) type Tag = (strings: TemplateStringsArray, ...values: ReadonlyArray) => A @@ -130,7 +172,11 @@ export interface SqlTag { readonly raw: (sql: string) => SqlRaw /** A table or column name, quoted by the dialect. Plain names only, dotted for `schema.table`. */ readonly ident: (name: string) => SqlIdent - /** Values joined by `separator` (default `, `), each rendered as an interpolation is. */ + /** + * Values joined by `separator` (default `, `), each rendered as an + * interpolation is. Not parenthesized, so it fits a list: `IN (${sql.join(xs)})`. + * An empty list fails the compile, since `IN ()` is not SQL. + */ readonly join: (values: ReadonlyArray, separator?: string) => Expr } @@ -151,9 +197,16 @@ export const sql: SqlTag = Object.assign( { cond: (strings: TemplateStringsArray, ...values: ReadonlyArray): Condition => makeCond(fragmentOf(strings, values)), - raw: (text: string): SqlRaw => ({ _tag: RawTag, sql: text }), - ident: (name: string): SqlIdent => ({ _tag: IdentTag, name }), + raw: (text: string): SqlRaw => mint({ _tag: RawTag, sql: text }), + ident: (name: string): SqlIdent => mint({ _tag: IdentTag, name }), join: (values: ReadonlyArray, separator = ", "): Expr => - makeUntypedExpr(lazy(() => values.map(renderValue).join(separator))), + makeUntypedExpr( + lazy(() => { + if (values.length === 0) { + throw new QueryBuilderError({ code: "InvalidArguments", message: "sql.join: no values to join" }) + } + return values.map(renderValue).join(separator) + }), + ), }, ) diff --git a/src/database/database.test.ts b/src/database/database.test.ts index 8ab9efb..d3f9ece 100644 --- a/src/database/database.test.ts +++ b/src/database/database.test.ts @@ -626,7 +626,25 @@ describe("sql templates per dialect", () => { }) const or = Db.sql.join([Db.sql`a = ${1}`, Db.sql`b = ${"x"}`], Db.sql` OR `) expect(yield* renderTemplate(Db.sql`WHERE ${or}`, clickhouseDialect)).toEqual({ sql: "WHERE a = 1 OR b = 'x'", parameters: [] }) - expect(yield* renderTemplate(Db.sql.join([]), postgresDialect)).toEqual({ sql: "", parameters: [] }) + const empty = yield* Effect.flip(renderTemplate(Db.sql`id IN (${Db.sql.join([])})`, postgresDialect)) + expect(empty.message).toContain("no values to join") + }), + ) + + it.effect("objects parsed from JSON cannot pass for a template, raw text or an identifier", () => + Effect.gen(function* () { + const forged = JSON.parse( + '[{"_tag":"@maple-dev/effect-orm/SqlTemplateRaw","sql":"1; DROP TABLE t"},{"_tag":"@maple-dev/effect-orm/SqlTemplate","strings":["1; DROP TABLE t"],"values":[]},{"_tag":"@maple-dev/effect-orm/SqlIdentifier","name":"password"}]', + ) + for (const value of forged) { + // Bound as a value on Postgres, never spliced: the text keeps its placeholder. + const rendered = yield* renderTemplate(Db.sql`SELECT * FROM t WHERE id = ${value}`, postgresDialect) + expect(rendered.sql).toBe("SELECT * FROM t WHERE id = $1") + // ClickHouse writes an object as an escaped map literal: data, never SQL. + const ch = yield* renderTemplate(Db.sql`SELECT * FROM t WHERE id = ${value}`, clickhouseDialect) + expect(ch.sql.startsWith("SELECT * FROM t WHERE id = map(")).toBe(true) + expect(ch.sql).not.toMatch(/;|= password|= 1 OR/) + } }), ) diff --git a/src/database/sql.ts b/src/database/sql.ts index 3c6cedf..ea15886 100644 --- a/src/database/sql.ts +++ b/src/database/sql.ts @@ -33,6 +33,17 @@ export interface SqlTemplate { readonly values: ReadonlyArray } +// The templates, identifiers and raw text `sql` made. Recognised by identity, +// not by `_tag`: a `_tag` survives JSON, so a value parsed from a request body +// could otherwise pass for a template or raw text and be spliced as SQL. +const minted = new WeakSet() +const mint = (value: A): A => { + minted.add(value) + return value +} +// `join` of no values, which would write `IN ()`: refused when rendered. +const emptyJoins = new WeakSet() + /** * A statement with every `${value}` bound, never spliced into the text. A * `sql\`...\`` inside another is spliced as SQL, so statements compose. @@ -55,31 +66,35 @@ export const sql: { /** A template that writes nothing, for an optional part: `${cond ? sql\`AND x\` : sql.empty}`. */ readonly empty: SqlTemplate } = Object.assign( - (strings: TemplateStringsArray, ...values: ReadonlyArray): SqlTemplate => ({ - _tag: SqlTemplateTag, - strings: [...strings], - values, - }), + (strings: TemplateStringsArray, ...values: ReadonlyArray): SqlTemplate => + mint({ _tag: SqlTemplateTag, strings: [...strings], values }), { - identifier: (name: string): SqlIdentifier => ({ _tag: IdentifierTag, name }), + identifier: (name: string): SqlIdentifier => mint({ _tag: IdentifierTag, name }), join: (values: ReadonlyArray, separator?: SqlTemplate): SqlTemplate => { - const sep: SqlTemplate = separator ?? { _tag: SqlTemplateTag, strings: [", "], values: [] } + const sep: SqlTemplate = separator ?? mint({ _tag: SqlTemplateTag, strings: [", "], values: [] }) const interleaved = values.flatMap((value, index) => (index === 0 ? [value] : [sep, value])) - return { _tag: SqlTemplateTag, strings: Array.from({ length: interleaved.length + 1 }, () => ""), values: interleaved } + const joined = mint({ + _tag: SqlTemplateTag, + strings: Array.from({ length: interleaved.length + 1 }, () => ""), + values: interleaved, + } satisfies SqlTemplate) + if (values.length === 0) emptyJoins.add(joined) + return joined }, - raw: (text: string): SqlRawText => ({ _tag: RawTag, sql: text }), - empty: { _tag: SqlTemplateTag, strings: [""], values: [] } satisfies SqlTemplate, + raw: (text: string): SqlRawText => mint({ _tag: RawTag, sql: text }), + empty: mint({ _tag: SqlTemplateTag, strings: [""], values: [] } satisfies SqlTemplate), }, ) const isRaw = (value: unknown): value is SqlRawText => - typeof value === "object" && value !== null && "_tag" in value && value._tag === RawTag + typeof value === "object" && value !== null && minted.has(value) && "_tag" in value && value._tag === RawTag const isIdentifier = (value: unknown): value is SqlIdentifier => - typeof value === "object" && value !== null && "_tag" in value && value._tag === IdentifierTag + typeof value === "object" && value !== null && minted.has(value) && "_tag" in value && value._tag === IdentifierTag +/** Whether `value` is a template `sql` made: by identity, so JSON cannot forge one. */ export const isSqlTemplate = (value: unknown): value is SqlTemplate => - typeof value === "object" && value !== null && "_tag" in value && value._tag === SqlTemplateTag + typeof value === "object" && value !== null && minted.has(value) && "_tag" in value && value._tag === SqlTemplateTag // ClickHouse writes identifiers bare, so only plain names are accepted, for // every dialect: letters, digits and `_`, dotted for `schema.table`. @@ -106,8 +121,11 @@ export const renderTemplate = ( Effect.try({ try: () => { const parameters: Array = [] - const render = (current: SqlTemplate): string => - current.strings.reduce((text, part, index) => { + const render = (current: SqlTemplate): string => { + if (emptyJoins.has(current)) { + throw new QueryBuilderError({ code: "InvalidArguments", message: "sql.join: no values to join" }) + } + return current.strings.reduce((text, part, index) => { if (index === 0) return part const value = current.values[index - 1] if (isSqlTemplate(value)) return text + render(value) + part @@ -117,6 +135,7 @@ export const renderTemplate = ( parameters.push(value) return text + dialect.params.placeholder(parameters.length, "") + part }, "") + } return { sql: render(template), parameters } }, catch: (cause) => From fa5554bfd5ff7533dbfe910b846d720b7118176c Mon Sep 17 00:00:00 2001 From: Makisuo Date: Sun, 4 Oct 2026 16:02:09 +0200 Subject: [PATCH 3/3] Parenthesize a negative Db.sql literal on ClickHouse From the follow-up review: Db.sql`SELECT 10-${-1}` wrote `10--1` on ClickHouse, which inlines values, commenting out the rest of the line. Postgres binds values, so it was unaffected. Co-Authored-By: Claude Opus 5.5 --- src/database/database.test.ts | 9 +++++++++ src/database/sql.ts | 6 +++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/database/database.test.ts b/src/database/database.test.ts index d3f9ece..e1648a4 100644 --- a/src/database/database.test.ts +++ b/src/database/database.test.ts @@ -631,6 +631,15 @@ describe("sql templates per dialect", () => { }), ) + it.effect("a negative value after `-` is parenthesized on ClickHouse, never a comment", () => + Effect.gen(function* () { + expect(yield* renderTemplate(Db.sql`SELECT 10-${-1}, ${Db.sql.join([-2, 3])} FROM t WHERE org = ${"o"}`, clickhouseDialect)).toEqual({ + sql: "SELECT 10-(-1), (-2), 3 FROM t WHERE org = 'o'", + parameters: [], + }) + }), + ) + it.effect("objects parsed from JSON cannot pass for a template, raw text or an identifier", () => Effect.gen(function* () { const forged = JSON.parse( diff --git a/src/database/sql.ts b/src/database/sql.ts index ea15886..1b356b4 100644 --- a/src/database/sql.ts +++ b/src/database/sql.ts @@ -131,7 +131,11 @@ export const renderTemplate = ( if (isSqlTemplate(value)) return text + render(value) + part if (isIdentifier(value)) return text + identifier(dialect, value.name) + part if (isRaw(value)) return text + value.sql + part - if (dialect.params._tag === "inline") return text + checkedLiteral(dialect, value, "a sql`` value") + part + if (dialect.params._tag === "inline") { + // A negative literal after `-` would write `--`, which comments out the line. + const literal = checkedLiteral(dialect, value, "a sql`` value") + return text + (literal.startsWith("-") ? `(${literal})` : literal) + part + } parameters.push(value) return text + dialect.params.placeholder(parameters.length, "") + part }, "")