Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/typed-errors-not-throws.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@maple-dev/effect-orm": patch
---

Values that cannot become SQL fail compilation with a `QueryBuilderError` (`InvalidLiteral`) instead of throwing: a bigint compared against a non-integer column, a non-string `like`/`notLike`/`ilike` pattern, `lit()` of anything but a string or number (it wrote `[object Object]`), a non-numeric arithmetic operand, an invalid `Date`, and an unparseable string or invalid `Date` for a Postgres timestamp. A non-expression in a `select` record is a `QueryBuilderDefect` naming the alias, like a malformed `orderBy`.
6 changes: 6 additions & 0 deletions src/ch/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -914,6 +914,12 @@ function compileInner<
// GROUP BY rules below know what each part reads and aggregates.
const selected = selectKeys.map((alias) => {
const expr = selectExprs[alias]
if (!isExprLike(expr)) {
const [sql, found] = track(() =>
fail(new QueryBuilderDefect({ message: `CHQuery: select(($) => ...) must map each alias to an expression; ${JSON.stringify(alias)} is not one` }), "NULL"),
)
return { alias, sql, found, column: undefined }
}
const fragment = expr.toFragment()
const [sql, found] = track(() => compileSqlFragment(fragment))
return { alias, sql, found, column: plainColumn(fragment) }
Expand Down
47 changes: 41 additions & 6 deletions src/ch/expr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,13 @@ export function toFragment(value: unknown): SqlFragment {
// be the dialect's own form (ClickHouse's is tz-less) rather than whatever
// `String(value)` produces.
if (DateTime.isDateTime(value)) return known(() => dateTimeLiteral(DateTime.toUtc(value)))
if (value instanceof Date) return known(() => dateTimeLiteral(DateTime.makeUnsafe(value)))
if (value instanceof Date) {
return known(() =>
Number.isNaN(value.getTime())
? fail(new QueryBuilderError({ code: "InvalidLiteral", message: "cannot write an invalid Date as a SQL literal" }), "NULL")
: dateTimeLiteral(DateTime.makeUnsafe(value)),
)
}
if (value === null) return raw("NULL")
if (typeof value === "bigint") return raw(String(value))
// Anything else has no literal here. `String(value)` used to be written as
Expand Down Expand Up @@ -342,9 +348,19 @@ const boundString = (value: string): SqlFragment =>
return bind !== undefined ? bind(value) : compile(str(value))
})

/** A LIKE pattern: an expression as it renders, a string as `boundString`. */
const likePattern = (pattern: string | Expr<string, any>): string =>
compile(typeof pattern === "string" ? boundString(pattern) : pattern.toFragment())
/** A LIKE pattern: an expression as it renders, a string as `boundString`. A
* pattern often comes from a request, so anything else is a failure. */
const likePattern = (pattern: string | Expr<string, any>): string => {
if (typeof pattern === "string") return compile(boundString(pattern))
if (isExprLike(pattern)) return compile(pattern.toFragment())
return fail(
new QueryBuilderError({
code: "InvalidLiteral",
message: `a LIKE pattern must be a string or a string expression, not ${describeUntyped(pattern)}`,
}),
"NULL",
)
}

const dateTimeLiteral = (value: DateTime.Utc): string =>
activeSqlSyntax()?.dateTimeLiteral(value) ?? compile(str(chDateTimeLiteral(value)))
Expand Down Expand Up @@ -407,7 +423,8 @@ const arith = <Result>(
rhs: number | null | Expr<number | null>,
lhsSchema?: Schema.Codec<any, any>,
): Expr<Result, any> => {
const rhsSchema = typeof rhs === "number" || rhs === null ? undefined : rhs.schema
const operand = typeof rhs === "number" || rhs === null || isExprLike(rhs)
const rhsSchema = isExprLike(rhs) ? rhs.schema : undefined
// `x / 1000000` is finite whenever `x` is. A literal below 1 in magnitude
// can overflow a large dividend (`1 / 5e-324` is `inf`), so only |d| >= 1
// keeps the strict codec — the same rule `Quotient` applies to the type.
Expand All @@ -422,6 +439,13 @@ const arith = <Result>(
const grouped = typeof rhs === "object" && rhs !== null && infix.has(rhs)
const expr = makeExpr(
known(() => {
// The operand is often a number from a request; anything else is a failure, not `'x'` or `NULL` in SQL.
if (!operand) {
return fail(
new QueryBuilderError({ code: "InvalidLiteral", message: `arithmetic operand must be a number or a numeric expression, not ${describeUntyped(rhs)}` }),
"NULL",
)
}
const right = compile(toFragment(rhs))
return `${compile(lhs)} ${op} ${grouped ? `(${right})` : right}`
}),
Expand Down Expand Up @@ -657,7 +681,18 @@ export function lit(value: string | number): Expr<string> | Expr<number> {
// a single `CH.lit("all")` in a SELECT costs the whole query its row schema,
// since derivation is all-or-nothing.
if (typeof value === "string") return makeExpr<string>(str(value), chString.schema)
return makeExpr<number>(typeof value === "number" ? numberFragment(value) : raw(String(value)), CHNumber as Schema.Codec<number, any>)
if (typeof value === "number") return makeExpr<number>(numberFragment(value), CHNumber as Schema.Codec<number, any>)
if (typeof value === "bigint") return makeExpr<number>(raw(String(value)), CHNumber as Schema.Codec<number, any>)
// `String(value)` used to be written as SQL, so `lit(obj)` emitted `[object Object]`.
return makeExpr<number>(
known(() =>
fail(
new QueryBuilderError({ code: "InvalidLiteral", message: `lit: cannot write ${describeUntyped(value)} as a SQL literal; pass a string or a number` }),
"NULL",
),
),
CHNumber as Schema.Codec<number, any>,
)
}

// Subquery expressions
Expand Down
35 changes: 35 additions & 0 deletions src/ch/failure.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,41 @@ const Events = CH.table("events", {

const PgEvents = PG.table("events", { columns: { id: PG.text, score: PG.nullable(PG.float8) }, primaryKey: ["id"] })

describe("values a request could supply fail as QueryBuilderError, never a throw", () => {
const PgTimed = PG.table("timed", { columns: { id: PG.text, at: PG.timestamptz }, primaryKey: ["id"] })
const failure = (effect: Effect.Effect<unknown, QueryBuilderError>) => {
const exit = Effect.runSyncExit(effect)
expect(Exit.isFailure(exit) && Cause.squash(exit.cause)).toBeInstanceOf(QueryBuilderError)
return Exit.isFailure(exit) ? String(Cause.squash(exit.cause)) : ""
}

it("a bigint compared against a String column", () => {
expect(failure(CH.compile(CH.from(Events).select(($) => ({ id: $.Id })).where(($) => [$.Id.eq(0n as never)])))).toMatch(/0n is not a valid value/)
})

it.each([[1], [{}], [new Map()]])("a LIKE pattern of %o", (pattern) => {
expect(failure(CH.compile(CH.from(Events).select(($) => ({ id: $.Id })).where(($) => [$.Name.like(pattern as never)])))).toMatch(/LIKE pattern/)
})

it.each([[{}], [Symbol("x")], [true]])("lit(%o)", (value) => {
expect(failure(CH.compile(CH.from(Events).select(() => ({ v: CH.lit(value as never) }))))).toMatch(/lit: cannot write/)
})

it.each([[undefined], ["5"], [{}], [new Date(Number.NaN)]])("an arithmetic operand of %o", (operand) => {
expect(failure(CH.compile(CH.from(Events).select(() => ({ v: CH.lit(1).add(operand as never) }))))).toMatch(/operand|invalid Date/)
})

it.each([[""], ["2026-13-45 99:99:99"], [new Date(Number.NaN)]])("a Postgres timestamp of %o", (value) => {
const query = PG.from(PgTimed).select(($) => ({ id: $.id })).where(($) => [$.at.gte(CH.param.dateTime("from"))])
expect(failure(PG.compile(query, { from: value as never }))).toMatch(/not a valid value/)
})

it("a non-expression in a select record is a defect: it is written in source", () => {
const exit = Effect.runSyncExit(CH.compile(CH.from(Events).select(() => ({ v: 1 as never }))))
expect(Exit.isFailure(exit) && Cause.squash(exit.cause)).toBeInstanceOf(QueryBuilderDefect)
})
})

describe("the builder never throws", () => {
it("builds a bad query without throwing; compiling it reports the failure", () => {
const build = () =>
Expand Down
1 change: 1 addition & 0 deletions src/ch/literal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const describe = (value: unknown): string => {
if (typeof value === "function") return "a function"
if (typeof value === "symbol") return "a symbol"
if (typeof value === "number" || typeof value === "boolean") return `${value}`
if (typeof value === "bigint") return `${value}n`
if (value instanceof Date) return "a Date"
if (Array.isArray(value)) return "an array"
if (typeof value === "object") return "an object"
Expand Down
11 changes: 9 additions & 2 deletions src/pg/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,13 @@ const PgTimestamptz: Schema.Codec<DateTime.Utc, Date | string> = Schema.Union([
* receives epoch milliseconds. Unlike a zoneless literal, an instant cannot be
* read in the session's time zone.
*/
const isValidDate = Schema.makeFilter((value: Date) => (Number.isNaN(value.getTime()) ? "an invalid Date" : undefined), {
title: "validDate",
})

// The checks sit on the value side, which is what encoding reads: a string or
// Date that is not an instant fails there as a schema issue, instead of reaching
// `format` as NaN and throwing from `toISOString`.
export const timestampLiteral = (format: (epochMillis: number) => string): Schema.Codec<unknown, unknown> =>
Schema.Union([
Schema.String.pipe(
Expand All @@ -78,14 +85,14 @@ export const timestampLiteral = (format: (epochMillis: number) => string): Schem
}),
),
Schema.String.pipe(
Schema.decodeTo(Schema.Date, {
Schema.decodeTo(Schema.Date.pipe(Schema.check(isValidDate)), {
decode: SchemaGetter.transform((value: string) => new Date(value)),
encode: SchemaGetter.transform((value: Date) => format(value.getTime())),
}),
),
Schema.String.pipe(
Schema.check(isTimestamp),
Schema.decodeTo(Schema.String, {
Schema.decodeTo(Schema.String.pipe(Schema.check(isTimestamp)), {
decode: SchemaGetter.transform((value: string) => value),
encode: SchemaGetter.transform((value: string) => format(Date.parse(pgTimestampToIso(value)))),
}),
Expand Down
14 changes: 11 additions & 3 deletions tests/fuzz/compile.fuzz.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const hostile: fc.Arbitrary<unknown> = fc.oneof(
fc.constantFrom<unknown>(Number.NaN, Infinity, -Infinity, -0, 2 ** 53, 1e308, 5e-324, new Date(Number.NaN), Symbol("s"), () => 1, undefined, null, "", "2026-13-45 99:99:99", { _brand: "Expr" }, { _brand: "Expr", toFragment: 1 }),
)

type HostileSite = "eq" | "neq" | "gt" | "in_" | "notIn" | "between" | "like" | "param.int" | "param.float" | "param.string" | "param.bool" | "param.dateTime" | "param.of" | "select"
type HostileSite = "lit" | "add" | "eq" | "neq" | "gt" | "in_" | "notIn" | "between" | "like" | "param.int" | "param.float" | "param.string" | "param.bool" | "param.dateTime" | "param.of" | "select"
export interface HostileInput {
readonly dialect: "clickhouse" | "postgres"
readonly column: OrderColumn
Expand All @@ -90,6 +90,8 @@ const hostileQuery = (ctx: CoreContext, input: HostileInput) => {
const params: Record<string, unknown> = { orgId: "org_1", h: value }
let q: any = ctx.from(ctx.orders)
if (site === "select") return { query: q.select(() => ({ id: CH.lit(1), bad: value })), params }
if (site === "lit") return { query: q.select(() => ({ id: CH.lit(value as never) })), params }
if (site === "add") return { query: q.select(($: any) => ({ id: $.Amount.add(value) })), params }
q = q.select(($: any) => ({ id: $.Id }))
if (site.startsWith("param.")) {
const kind = site.slice("param.".length)
Expand All @@ -106,7 +108,7 @@ const hostileInput = (dialect: "clickhouse" | "postgres"): fc.Arbitrary<HostileI
fc.record({
dialect: fc.constant(dialect),
column: fc.constantFrom(...(Object.keys(ORDER_COLUMNS) as OrderColumn[])),
site: fc.constantFrom<HostileSite>("eq", "neq", "gt", "in_", "notIn", "between", "like", "param.int", "param.float", "param.string", "param.bool", "param.dateTime", "param.of", "select"),
site: fc.constantFrom<HostileSite>("lit", "add", "eq", "neq", "gt", "in_", "notIn", "between", "like", "param.int", "param.float", "param.string", "param.bool", "param.dateTime", "param.of", "select"),
value: hostile,
})

Expand All @@ -123,7 +125,13 @@ const neverThrows = (ctx: CoreContext, input: HostileInput) => {
})
}
if (Exit.isFailure(exit)) {
const typed = exit.cause.reasons.every((reason) => reason._tag === "Fail" && String((reason as any).error?._tag).endsWith("QueryBuilderError"))
// src/ch/errors.ts: a value a request could supply fails with a QueryBuilderError; what is only ever
// written in source (the shape of a select record) may die with a QueryBuilderDefect. Nothing else.
const sourceSite = input.site === "select"
const typed = exit.cause.reasons.every((reason) => {
const tag = String(((reason as any).error ?? (reason as any).defect)?._tag)
return (reason._tag === "Fail" && tag.endsWith("QueryBuilderError")) || (sourceSite && reason._tag === "Die" && tag.endsWith("QueryBuilderDefect"))
})
if (!typed) throw new FuzzFailure("compile died or failed with something other than a QueryBuilderError", { cause: String(exit.cause) })
}
expect(true).toBe(true)
Expand Down
48 changes: 4 additions & 44 deletions tests/fuzz/known-issues.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,51 +58,11 @@ export const knownLiteralIssue = (input: LiteralInput): string | undefined =>

import type { HostileInput } from "./compile.fuzz.test"

const containsBigInt = (value: unknown, seen = new Set<unknown>()): boolean => {
if (typeof value === "bigint") return true
if (value === null || typeof value !== "object" || seen.has(value)) return false
seen.add(value)
try {
return Reflect.ownKeys(value).some((key) => containsBigInt((value as any)[key], seen))
} catch {
return false
}
}

const hostileIssues: ReadonlyArray<KnownIssue<HostileInput>> = [
{
id: "describe-bigint",
reason:
"literal.ts `describe()` formats an unexpected value with JSON.stringify, which throws on a bigint, so " +
"`$.OrgId.eq(0n)` dies with `TypeError: Do not know how to serialize a BigInt` instead of an InvalidLiteral error.",
matches: ({ value }) => containsBigInt(value),
},
{
id: "like-non-string-pattern",
reason:
"`like`/`notLike`/`ilike` call `pattern.toFragment()` on anything that is not a string, so a number, " +
"object or Map pattern dies with `TypeError: pattern.toFragment is not a function`.",
matches: ({ site, value }) => site === "like" && typeof value !== "string",
},
{
id: "select-non-expression",
reason:
"A select callback returning a non-expression (`{ bad: 1 }`, `{ bad: undefined }`) dies with " +
"`TypeError: selectExprs[alias].toFragment is not a function` instead of a QueryBuilderError naming the alias.",
matches: ({ site, value }) =>
site === "select" && !(value !== null && typeof value === "object" && typeof (value as any).toFragment === "function"),
},
{
id: "pg-timestamp-invalid-string-dies",
reason:
"The Postgres timestamp literal codec formats an unparseable string or invalid Date with toISOString(), which " +
"throws `RangeError: Invalid time value` inside the transform: compile dies instead of failing InvalidLiteral.",
matches: ({ dialect, site, column, value }) =>
dialect === "postgres" &&
(site === "param.dateTime" || (column === "Created" && (site === "param.of" || !site.startsWith("param.")))) &&
(typeof value === "string" || value instanceof Date) &&
Number.isNaN(new Date(value as string).getTime()),
},




]

export const knownHostileIssue = (input: HostileInput): string | undefined =>
Expand Down
3 changes: 3 additions & 0 deletions tests/fuzz/regressions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ export const pinned: Record<string, ReadonlyArray<unknown>> = {
{ dialect: "clickhouse", column: "OrgId", site: "eq", value: 0n },
{ dialect: "clickhouse", column: "OrgId", site: "like", value: {} },
{ dialect: "clickhouse", column: "OrgId", site: "select", value: Number.NaN },
{ dialect: "clickhouse", column: "OrgId", site: "lit", value: 0n },
{ dialect: "clickhouse", column: "OrgId", site: "add", value: undefined },
{ dialect: "clickhouse", column: "OrgId", site: "add", value: new Date(Number.NaN) },
],
"hostile values are refused, never thrown (postgres)": [
{ dialect: "postgres", column: "OrgId", site: "param.dateTime", value: "" },
Expand Down
Loading