From 974f822d34c16e2e744e61cf8b5b8164854397bb Mon Sep 17 00:00:00 2001 From: Makisuo Date: Fri, 23 Oct 2026 02:07:23 +0200 Subject: [PATCH] fix(postgres): type the two placeholders Postgres would mistype Postgres types an untyped `$n` from its context, which went wrong twice: - a `param.of(type)` selected on its own has no context, so it bound as text: a boolean came back as 'false', an array as '{}', and decoding failed. It is now `CAST($n AS )` in the select list. - a fractional value compared with an integer column was typed int8 and failed with `invalid input syntax for type bigint`. It now binds as `$n::numeric`. Nothing else is cast, so whole numbers and params compared with columns keep using indexes, `real` columns still match, and an insert of a fraction into an integer column is still refused rather than rounded. `param.of` kinds were keyed by codec alone, and types share codecs (`PG.int8`/`PG.float8`), so a kind could carry another type's SQL; they are keyed by codec and SQL type now. Fuzz model: ClickHouse float `%` is not IEEE fmod, and the matcher for the upstream ClickHouse and()-absorption bug covers nested ORs. Co-Authored-By: Claude Opus 5.5 --- .changeset/pg-typed-placeholders.md | 5 +++ src/ch/compile.ts | 19 +++++++++-- src/ch/expr.ts | 20 +++++++++--- src/ch/param.ts | 24 +++++++++++--- src/pg/bound-literals.test.ts | 49 +++++++++++++++++++++++++++++ tests/fuzz/known-issues.ts | 33 +++++-------------- tests/fuzz/model.ts | 2 ++ 7 files changed, 115 insertions(+), 37 deletions(-) create mode 100644 .changeset/pg-typed-placeholders.md diff --git a/.changeset/pg-typed-placeholders.md b/.changeset/pg-typed-placeholders.md new file mode 100644 index 0000000..9818989 --- /dev/null +++ b/.changeset/pg-typed-placeholders.md @@ -0,0 +1,5 @@ +--- +"@maple-dev/effect-orm": patch +--- + +Two Postgres values the server typed wrong from context now compile correctly. A `param.of(type)` selected on its own is written `CAST($n AS )`, so a boolean or array comes back as itself instead of text that fails to decode. A fractional value compared with an integer column binds as `$n::numeric` instead of failing with `invalid input syntax for type bigint`. Nothing else is cast: whole numbers, params compared with columns, inserts and `real` columns keep untyped placeholders (and their indexes). `param.of` also no longer confuses two types that share a codec (`int8` and `float8`). diff --git a/src/ch/compile.ts b/src/ch/compile.ts index ef9cff2..0aaaac1 100644 --- a/src/ch/compile.ts +++ b/src/ch/compile.ts @@ -21,7 +21,7 @@ import { splitTerminalClauses } from "../sql/terminal-clauses" import { activeLiteralBinder, withLiteralBinder } from "../sql/literal-binder" import { compileQuery, type SqlQuery } from "../sql/sql-query" import { track, untracked, type RenderTrack } from "../sql/render-tracker" -import { PARAM_MARKER_PREFIX, PARAM_PLACEHOLDER_PATTERN, param, paramSchema, type ParamKind } from "./param" +import { PARAM_MARKER_PREFIX, PARAM_PLACEHOLDER_PATTERN, param, paramColumnTypeOf, paramSchema, type ParamKind } from "./param" import { mergeResultSchemas } from "./define-fn" import { encodeValue } from "./literal" import { checkedLiteral, clickhouseDialect, currentDialect, withDialect, type Dialect } from "./dialect" @@ -922,7 +922,7 @@ function compileInner< } const fragment = expr.toFragment() const [sql, found] = track(() => compileSqlFragment(fragment)) - return { alias, sql, found, column: plainColumn(fragment) } + return { alias, sql: castSelectedParam(expr, sql), found, column: plainColumn(fragment) } }) const where = presentWhere.map((c) => raw(rowFilter("WHERE", c))) const having = (state.havingFn ? state.havingFn($) : []) @@ -1263,6 +1263,21 @@ function derivedLiteralSchema(schema: Schema.Codec): Schema.Codec { + const sqlType = paramColumnTypeOf(expr)?.sql + // Only a lowercase type name: that is how Postgres types are spelled, and a + // ClickHouse type (`String`, `Nullable(…)`) handed to a Postgres query would + // otherwise become a cast the server rejects. + return sqlType && /^[a-z_]/.test(sqlType) && currentDialect().params._tag === "bind" ? `CAST(${sql} AS ${sqlType})` : sql +} + /** The aliases a query selects, read without compiling it; `undefined` before `select()`. */ export function selectedAliasesOf(query: CHQuery): ReadonlyArray | undefined { const exprs = selectExprsOf(query) diff --git a/src/ch/expr.ts b/src/ch/expr.ts index 68d238a..99773df 100644 --- a/src/ch/expr.ts +++ b/src/ch/expr.ts @@ -338,17 +338,27 @@ const describeUntyped = (value: unknown): string => * A value compared against a column. On ClickHouse an array or map literal is * cast to the column's type: otherwise its element type is inferred from the * values (`[0, 4294967296]` is Array(UInt64)), which may not compare with the - * column at all. A bound value needs no cast; the driver sends it typed. + * column at all. */ const comparisonLiteral = (columnType: CHType, value: unknown, column: string): string => { const literal = encodeColumnLiteral(columnType, value, column) - const compound = columnType._tag === "Array" || columnType._tag === "Map" // Outside a compile, ClickHouse is the default dialect. - return compound && activeLiteralBinder() === undefined && (activeSqlSyntax()?.dialect ?? "clickhouse") === "clickhouse" - ? `CAST(${literal} AS ${columnType.sql})` - : literal + const dialect = activeSqlSyntax()?.dialect ?? "clickhouse" + const bound = activeLiteralBinder() !== undefined + if (!bound && dialect === "clickhouse" && (columnType._tag === "Array" || columnType._tag === "Map")) { + return `CAST(${literal} AS ${columnType.sql})` + } + // Postgres types a bound value from the column, and an integer column turns + // `9.75` into `invalid input syntax for type bigint`. Only a fraction is + // cast: a whole number left untyped keeps the comparison on the index. + if (bound && dialect === "postgres" && POSTGRES_INTEGERS.has(columnType.sql) && typeof value === "number" && !Number.isInteger(value)) { + return `${literal}::numeric` + } + return literal } +const POSTGRES_INTEGERS = new Set(["int2", "int4", "int8", "smallint", "integer", "bigint"]) + /** A number as SQL that reads back exactly. A subnormal goes through the dialect: ClickHouse cannot parse one as written. */ const numberFragment = (value: number): SqlFragment => isSubnormal(value) ? known(() => (activeSqlSyntax()?.literal ?? sqlLiteral)(value, "a number literal")) : raw(numberText(value)) diff --git a/src/ch/param.ts b/src/ch/param.ts index 420e611..0239cc8 100644 --- a/src/ch/param.ts +++ b/src/ch/param.ts @@ -95,13 +95,15 @@ const unresolvedSql = (name: string): SqlFragment => function makeParamMarker( name: N, fragment: SqlFragment, - schema?: Schema.Codec, + schema: Schema.Codec | undefined, + kind: ParamKind, ): ParamMarker { const raise = (): Condition => makeCond(unresolvedSql(name)) const raiseExpr = (): Expr => makeUntypedExpr(unresolvedSql(name)) return { _brand: "Expr" as const, _paramName: name, + _paramKind: kind, schema, toFragment: () => fragment, eq: raise, @@ -166,21 +168,33 @@ const makeParam = const fragment = isValidParamName(name) ? raw(paramPlaceholder(kind, name)) : known(() => fail(badParamName(name), "NULL")) - return makeParamMarker(name, fragment, schema) as ParamMarker + return makeParamMarker(name, fragment, schema, kind) as ParamMarker } -const customKinds = new WeakMap, ParamKind>() +// Keyed by codec *and* SQL type: types share codecs (`PG.int8` and `PG.float8` +// both encode with one number codec), and a dialect that types its +// placeholders needs the kind to name one SQL type. +const customKinds = new WeakMap, Map>() let nextCustomKind = 0 +/** The column type a `param.of` kind was made from, for a dialect that types its placeholders. */ +const kindColumnTypes = new Map>() const kindFor = (type: CHType): ParamKind => { - const existing = customKinds.get(type.literalSchema) + let bySql = customKinds.get(type.literalSchema) + if (bySql === undefined) customKinds.set(type.literalSchema, (bySql = new Map())) + const existing = bySql.get(type.sql) if (existing !== undefined) return existing const kind = `custom${nextCustomKind++}` - customKinds.set(type.literalSchema, kind) + bySql.set(type.sql, kind) paramTypes.set(kind, type.literalSchema) + kindColumnTypes.set(kind, type) return kind } +/** The column type of a `param.of` placeholder; `undefined` for anything else. */ +export const paramColumnTypeOf = (expr: unknown): CHType | undefined => + typeof expr === "object" && expr !== null && "_paramKind" in expr ? kindColumnTypes.get((expr as { readonly _paramKind: ParamKind })._paramKind) : undefined + export const param = { /** Resolved from a string; emitted as an escaped SQL literal. */ string: makeParam("string", T.string.schema), diff --git a/src/pg/bound-literals.test.ts b/src/pg/bound-literals.test.ts index b45b6f5..b9571a4 100644 --- a/src/pg/bound-literals.test.ts +++ b/src/pg/bound-literals.test.ts @@ -99,3 +99,52 @@ describe("orderBy params", () => { expect(PG.compileUnsafe(byParam, { fallback: "z" }).parameters).toEqual(["z"]) }) }) + +describe("typed placeholders", () => { + const Scores = PG.table("scores", { columns: { id: PG.int8, ok: PG.bool, tags: PG.array(PG.text) }, primaryKey: ["id"] }) + const run = (compiled: PG.CompiledQuery) => + Effect.promise(() => db.query>(compiled.sql, [...compiled.parameters])).pipe( + Effect.flatMap((result) => compiled.decodeRows(result.rows)), + ) + + it.effect("a param.of in a select list binds as its own type, not text", () => + Effect.gen(function* () { + yield* Effect.promise(() => db.exec(`CREATE TABLE IF NOT EXISTS scores (id int8 PRIMARY KEY, ok boolean, tags text[]); INSERT INTO scores VALUES (1, true, '{a}') ON CONFLICT DO NOTHING`)) + const compiled = PG.compileUnsafe( + PG.from(Scores).select(() => ({ flag: CH.param.of(PG.bool, "flag"), list: CH.param.of(PG.array(PG.text), "list") })), + { flag: false, list: [] }, + ) + expect(compiled.sql).toContain(`CAST($1 AS boolean) AS "flag"`) + expect(compiled.sql).toContain(`CAST($2 AS text[]) AS "list"`) + expect(yield* run(compiled)).toEqual([{ flag: false, list: [] }]) + // A ClickHouse type has no Postgres name to cast to; it is left as it was. + expect(PG.compileUnsafe(PG.from(Scores).select(() => ({ s: CH.param.of(CH.string, "s") })), { s: "a" }).sql).toContain(`$1 AS "s"`) + }), + ) + + it.effect("a fraction compared with an integer column binds as numeric; a whole number stays uncast", () => + Effect.gen(function* () { + const compiled = PG.compileUnsafe(PG.from(Scores).select("id").where(($) => [$.id.gt(0.5), $.id.lt(2)]), {}) + expect(compiled.sql).toContain(`"id" > $1::numeric`) + expect(compiled.sql).toMatch(/"id" < \$2(?!:)/) + expect(yield* run(compiled)).toEqual([{ id: 1 }]) + }), + ) + + it.effect("a param compared with a column stays untyped, and a float4 fraction still matches", () => + Effect.gen(function* () { + const Reals = PG.table("reals", { columns: { id: PG.int8, r: PG.float4 }, primaryKey: ["id"] }) + yield* Effect.promise(() => db.exec(`CREATE TABLE IF NOT EXISTS reals (id int8 PRIMARY KEY, r real); INSERT INTO reals VALUES (1, 0.1) ON CONFLICT DO NOTHING`)) + const byParam = PG.compileUnsafe(PG.from(Scores).select("id").where(($) => [$.id.eq(CH.param.of(PG.int8, "v"))]), { v: 1 }) + expect(byParam.sql).toMatch(/"id" = \$1(?!:)/) + const real = PG.compileUnsafe(PG.from(Reals).select("id").where(($) => [$.r.eq(0.1)]), {}) + expect(real.sql).toMatch(/"r" = \$1(?!:)/) + expect(yield* run(real)).toEqual([{ id: 1 }]) + }), + ) + + it("inserting a fraction into an integer column is not cast, so it is not rounded", () => { + const compiled = PG.compileUnsafe(PG.insertInto(Scores).values([{ id: 9.75, ok: true, tags: [] }])) + expect(compiled.sql).not.toContain("::numeric") + }) +}) diff --git a/tests/fuzz/known-issues.ts b/tests/fuzz/known-issues.ts index 54647cc..52fec6a 100644 --- a/tests/fuzz/known-issues.ts +++ b/tests/fuzz/known-issues.ts @@ -26,15 +26,7 @@ export interface LiteralInput { const literalIssues: ReadonlyArray> = [ - { - id: "pg-untyped-select-param", - reason: - "A param in a Postgres SELECT list is bound untyped (`$1 AS \"p\"`), so the server infers `text` and a " + - "boolean comes back as `'false'`, an array as `'{…}'`, neither of which the column's codec decodes. " + - "Writing `$1::` would fix it.", - matches: ({ literal, path }) => - literal.dialect === "postgres" && path === "select" && (literal.type.sql === "boolean" || literal.type._tag === "Array"), - }, + ] /** The id of the open issue this input would hit, if any. */ @@ -83,33 +75,24 @@ const planIssues: ReadonlyArray> = [ { id: "clickhouse-server-and-absorption", reason: - "A ClickHouse 25.8 server bug, not the builder: `WHERE a AND ((b AND a) OR a)` fails with " + + "A ClickHouse 25.8 server bug, not the builder: `WHERE a AND ((b AND a) OR a)` or `a AND ((a OR a) OR a)` fails with " + "`Number of arguments for function \"and\" should be at least 2` — the common-expression rewrite leaves a " + "one-argument and(). `SETTINGS optimize_extract_common_expressions = 0` avoids it.", matches: ({ target, plan }) => { if (target === "postgres") return false const top = conjuncts(plan.where) + const disjuncts = (c: any): Array => (c?.k === "or" ? c.cs.flatMap(disjuncts) : [c]) + // Fails when every disjunct of a nested OR contains a top-level conjunct `w` (as `w`, or inside an AND): + // `w AND ((x AND w) OR w)`, `w AND ((w OR w) OR w)`. `w AND (w OR w)` and `w AND ((x AND w) OR y)` run. return [...nodes(plan)].some( (node) => node.k === "or" && - top.some( - (w) => - (node.cs as Array).some((d) => same(d, w)) && - (node.cs as Array).some((d) => d?.k === "and" && conjuncts(d.cs).some((x) => same(x, w))), - ), + (node.cs as Array).some((d) => d?.k === "or" || d?.k === "and") && + top.some((w) => disjuncts(node).every((d) => same(d, w) || (d?.k === "and" && conjuncts(d.cs).some((x) => same(x, w))))), ) }, }, - { - id: "pg-fraction-bound-to-integer", - reason: - "On Postgres a comparison value is bound as a parameter, and the server types it from the integer side: " + - "`\"Amount\" > $1` with 9.75 fails `invalid input syntax for type bigint`. ClickHouse inlines the literal " + - "and compares correctly. The builder accepts the value (int8's codec is any finite number).", - matches: ({ target, plan }) => - target === "postgres" && - [...nodes(plan)].some((node) => node.k === "value" && typeof node.v === "number" && !Number.isInteger(node.v)), - }, + ] diff --git a/tests/fuzz/model.ts b/tests/fuzz/model.ts index 2f7f305..3136d52 100644 --- a/tests/fuzz/model.ts +++ b/tests/fuzz/model.ts @@ -158,6 +158,8 @@ const evalExpr = (env: Env, row: Row, e: Expr): V => { if (env.target !== "postgres") { const kinds = new Set([chKind(e.l), chKind(e.r)]) if (kinds.has("unsigned") && kinds.has("signed")) skip("ClickHouse % across signed and unsigned") + // Not IEEE fmod: `3 % (27 / 99)` is 0 on ClickHouse, 2.2e-16 by fmod. + if (kinds.has("float")) skip("ClickHouse % of a float") } // Both databases truncate towards zero, like JS `%`. return l % r