From a9ab64106d4daa4a257633139532323795237194 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Wed, 21 Oct 2026 16:25:55 +0200 Subject: [PATCH 1/2] fix(clickhouse): refuse values with no SQL literal instead of writing String(value) A value compared against, or passed to, an expression with no column type (a defineFn result, untypedExpr, an untyped dynamicColumn) fell back to raw(String(value)) when it was not a string, number, boolean or date. An array or object that arrived from a request untyped became SQL. It now fails compilation with QueryBuilderError InvalidLiteral; null and bigint keep their literals. Co-Authored-By: Claude Opus 5.5 --- .../untyped-literal-refuses-non-scalars.md | 5 +++ docs/expressions.md | 13 +++--- src/ch/expr.ts | 17 ++++++- src/ch/failure.test.ts | 44 +++++++++++++++++++ 4 files changed, 73 insertions(+), 6 deletions(-) create mode 100644 .changeset/untyped-literal-refuses-non-scalars.md diff --git a/.changeset/untyped-literal-refuses-non-scalars.md b/.changeset/untyped-literal-refuses-non-scalars.md new file mode 100644 index 0000000..b1fecd0 --- /dev/null +++ b/.changeset/untyped-literal-refuses-non-scalars.md @@ -0,0 +1,5 @@ +--- +"@maple-dev/effect-orm": patch +--- + +Refuse values with no SQL literal instead of writing `String(value)`. A value compared against, or passed to, an expression with no column type (a `defineFn` result, `untypedExpr`, an untyped `dynamicColumn`) used to be written raw when it was not a string, number, boolean or date: an array or object from a request became part of the SQL (`toFloat64OrNull(x) > 1) OR (1=1`). It now fails compilation with a `QueryBuilderError` (`InvalidLiteral`). `null` is written as `NULL` and a bigint as its digits, as before. diff --git a/docs/expressions.md b/docs/expressions.md index 3f506eb..79a41e9 100644 --- a/docs/expressions.md +++ b/docs/expressions.md @@ -45,11 +45,14 @@ $.Count.eq("lots") // QueryBuilderError { code: "InvalidLiteral" }: column Count: string "lots" is not a valid value ``` -Expressions with no type to read — `untypedExpr`, an untyped `dynamicColumn` — fall back to -guessing from the JavaScript value, which handles strings, numbers, booleans and dates but -nothing structured. - -_(Backed by `src/ch/literal.test.ts`.)_ +Expressions with no type to read — `untypedExpr`, an untyped `dynamicColumn`, a function +declared with `defineFn` — fall back to guessing from the JavaScript value, which handles +strings, numbers, bigints, booleans, dates and `null`. Anything structured (an array, an +object) or `undefined` fails compilation with a `QueryBuilderError` +(`InvalidLiteral`) rather than being written as `String(value)`, so a value that arrives from +a request untyped can never become SQL. + +_(Backed by `src/ch/literal.test.ts` and `src/ch/failure.test.ts`.)_ ## Comparisons diff --git a/src/ch/expr.ts b/src/ch/expr.ts index ab439be..74eb691 100644 --- a/src/ch/expr.ts +++ b/src/ch/expr.ts @@ -307,9 +307,24 @@ export function toFragment(value: unknown): SqlFragment { // `String(value)` produces. if (DateTime.isDateTime(value)) return known(() => dateTimeLiteral(DateTime.toUtc(value))) if (value instanceof Date) return known(() => dateTimeLiteral(DateTime.makeUnsafe(value))) - return raw(String(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 + // SQL, so an array or object from a request became part of the query. + return known(() => + fail( + new QueryBuilderError({ + code: "InvalidLiteral", + message: `cannot write ${describeUntyped(value)} as a SQL literal; compare against a typed column or pass a string, number, boolean or date`, + }), + "NULL", + ), + ) } +const describeUntyped = (value: unknown): string => + Array.isArray(value) ? "an array" : value === undefined ? "undefined" : `a ${typeof value}` + /** A value with no column type to encode it, in the active dialect's syntax. * ClickHouse writes booleans as `1`/`0`, which is also the rendering outside * a compile. */ diff --git a/src/ch/failure.test.ts b/src/ch/failure.test.ts index 0cff3a4..0883baf 100644 --- a/src/ch/failure.test.ts +++ b/src/ch/failure.test.ts @@ -31,6 +31,50 @@ describe("the builder never throws", () => { expect(error.message).toMatch(/limit\(-1\)/) }) + // A value with no column type to encode it was written as `String(value)`: + // an array or object from a request became SQL. It is a typed failure now. + it.each([ + ["an array", ["1) OR (1=1"]], + ["an object", { toString: (): string => "1) OR (1=1" }], + ["a symbol", Symbol("x")], + ["a function", () => "1"], + ])("refuses %s compared against an untyped expression", (_label, value) => { + const toFloat = CH.defineFn<[CH.Expr], number>("toFloat64OrNull", CH.float64) + const query = CH.from(Events) + .select(($) => ({ id: $.Id })) + .where(($) => [toFloat($.Name).gt(value as never)]) + const error = Effect.runSync(Effect.flip(CH.compile(query))) + expect(error).toBeInstanceOf(QueryBuilderError) + expect(error.code).toBe("InvalidLiteral") + }) + + it.each([ + ["an object", { toString: (): string => "1) OR (1=1" }], + ["an array", ["1) OR (1=1"]], + ["undefined", undefined], + ])("refuses %s as a function argument", (_label, value) => { + const twice = CH.defineFn<[unknown], number>("twice", CH.float64) + const query = CH.from(Events).select(() => ({ n: twice(value) })) + const error = Effect.runSync(Effect.flip(CH.compile(query))) + expect(error.code).toBe("InvalidLiteral") + }) + + it("still writes null, a bigint and the scalars it always wrote", () => { + const toFloat = CH.defineFn<[CH.Expr], number>("toFloat64OrNull", CH.float64) + const fn = CH.defineFn<[unknown], number>("f", CH.float64) + const sql = CH.compileUnsafe( + CH.from(Events) + .select(() => ({ a: fn(null), b: fn(12345678901234567890n), c: fn("x'y"), d: fn(1.5), e: fn(true) })) + .where(($) => [toFloat($.Name).gt(3)]), + ).sql + expect(sql).toContain("f(NULL) AS a") + expect(sql).toContain("f(12345678901234567890) AS b") + expect(sql).toContain("f('x\\'y') AS c") + expect(sql).toContain("f(1.5) AS d") + expect(sql).toContain("f(1) AS e") + expect(sql).toContain("toFloat64OrNull(events.Name) > 3") + }) + it("dies on a misuse no input could fix", () => { const query = CH.from(Events).select(($) => ({ id: $.Id })).where(($) => [$.Id.eq(CH.param.string("bad__name"))]) const exit = Effect.runSyncExit(CH.compile(query, { bad__name: "x" })) From 32d91da94bc512b31cfb951adccbf479bee161b4 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Wed, 21 Oct 2026 16:32:22 +0200 Subject: [PATCH 2/2] docs(expressions): name the refused values and both undefined error codes Co-Authored-By: Claude Opus 5.5 --- docs/expressions.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/expressions.md b/docs/expressions.md index 79a41e9..25a7fb2 100644 --- a/docs/expressions.md +++ b/docs/expressions.md @@ -47,10 +47,12 @@ $.Count.eq("lots") Expressions with no type to read — `untypedExpr`, an untyped `dynamicColumn`, a function declared with `defineFn` — fall back to guessing from the JavaScript value, which handles -strings, numbers, bigints, booleans, dates and `null`. Anything structured (an array, an -object) or `undefined` fails compilation with a `QueryBuilderError` -(`InvalidLiteral`) rather than being written as `String(value)`, so a value that arrives from -a request untyped can never become SQL. +strings, numbers, bigints, booleans, dates and `null`; a string is written as an escaped +literal. Anything structured (an array, an object), a symbol or a function fails compilation +with a `QueryBuilderError` (`InvalidLiteral`) rather than being written as `String(value)`, +so an unsupported value that arrives untyped is never written as raw SQL. `undefined` fails +too: as a function argument with `InvalidLiteral`, and compared against an expression +(`.gt(undefined)`) with `InvalidArguments`, like any comparison with a missing value. _(Backed by `src/ch/literal.test.ts` and `src/ch/failure.test.ts`.)_