fix(postgres): type the two placeholders Postgres would mistype

.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 <type>)`, 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`).

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<any, any>): Schema.Codec<any,
 	return schema
 }
 
+/**
+ * A `param.of(type)` selected on its own, cast to its type where the dialect
+ * binds. A bound placeholder has no type of its own and a select list gives it
+ * none, so Postgres sends it back as text: a boolean as `'false'`, an array as
+ * `'{}'`, which the type's codec rejects. Anywhere else the column it is
+ * compared with types it, and a cast there could only cost an index.
+ */
+const castSelectedParam = (expr: unknown, sql: string): string => {
+	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<any, any, any>): ReadonlyArray<string> | undefined {
 	const exprs = selectExprsOf(query)

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<string, any, any>, 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))

src/ch/param.ts

@@ -95,13 +95,15 @@ const unresolvedSql = (name: string): SqlFragment =>
 function makeParamMarker<N extends string, T>(
 	name: N,
 	fragment: SqlFragment,
-	schema?: Schema.Codec<T, any>,
+	schema: Schema.Codec<T, any> | undefined,
+	kind: ParamKind,
 ): ParamMarker<N, T> {
 	const raise = (): Condition<any> => makeCond(unresolvedSql(name))
 	const raiseExpr = (): Expr<any, any> => 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<N, T>(name, fragment, schema) as ParamMarker<N, T, V>
+		return makeParamMarker<N, T>(name, fragment, schema, kind) as ParamMarker<N, T, V>
 	}
 
-const customKinds = new WeakMap<Schema.Codec<any, any>, 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<Schema.Codec<any, any>, Map<string, ParamKind>>()
 let nextCustomKind = 0
+/** The column type a `param.of` kind was made from, for a dialect that types its placeholders. */
+const kindColumnTypes = new Map<ParamKind, CHType<string, any, any>>()
 
 const kindFor = (type: CHType<string, any, any>): 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<string, any, any> | 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>("string", T.string.schema),

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<any>) =>
+		Effect.promise(() => db.query<Record<string, unknown>>(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")
+	})
+})

tests/fuzz/known-issues.ts

@@ -26,15 +26,7 @@ export interface LiteralInput {
 
 const literalIssues: ReadonlyArray<KnownIssue<LiteralInput>> = [
 
-	{
-		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::<type>` 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<KnownIssue<PlanInput>> = [
 	{
 		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<any> => (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<any>).some((d) => same(d, w)) &&
-							(node.cs as Array<any>).some((d) => d?.k === "and" && conjuncts(d.cs).some((x) => same(x, w))),
-					),
+					(node.cs as Array<any>).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)),
-	},
+
 
 ]
 

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