Skip to content

Commit 28fc791

Browse files
authored
Merge pull request #134 from MapleTechLabs/t3/fix-clickhouse-fuzzer-invalid-expressions
test(fuzz): fix nightly soak's division-by-zero and ClickHouse 26.2 modulo failures
2 parents d44b38e + ba57dba commit 28fc791

4 files changed

Lines changed: 76 additions & 29 deletions

File tree

‎tests/fuzz/compile.fuzz.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ const stringsIn = (value: unknown): Array<string> =>
2424
const serialize = (value: unknown) => JSON.stringify(value, (_, v) => (typeof v === "bigint" ? `${v}n` : v instanceof Date ? `Date(${v.getTime()})` : v))
2525

2626
const compiles = (dialect: FuzzDialect, input: Plan) => {
27-
const known = knownPlanIssue({ target: dialect.name, plan: input })
27+
const known = knownPlanIssue({ target: dialect.name, plan: input, executed: false })
2828
if (known !== undefined) skipKnown(known)
2929
const ctx = scopeContext(dialect)
3030
const first = compilePlan(dialect, ctx, input)

‎tests/fuzz/known-issues.ts‎

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,38 @@ export const knownHostileIssue = (input: HostileInput): string | undefined =>
4141

4242
// Query plans (queries.fuzz.test.ts)
4343

44-
import type { Plan, UnionPlan } from "./plan"
44+
import { foldConstant } from "./model"
45+
import type { Expr, Plan, UnionPlan } from "./plan"
4546

4647
export interface PlanInput {
4748
/** The target name (`dialects/*.ts`), or the dialect name for compile-only properties. */
4849
readonly target: string
4950
readonly plan: Plan
51+
/** Whether the plan runs on a database: a server bug only matters to properties that run it. */
52+
readonly executed: boolean
5053
}
5154

52-
const planIssues: ReadonlyArray<KnownIssue<PlanInput>> = []
55+
/** A `%` anywhere in the plan whose divisor folds to -2, -4, -8, …: the divisor is always the node's `r`. */
56+
const negativePowerOfTwoModulo = (value: unknown): boolean => {
57+
if (Array.isArray(value)) return value.some(negativePowerOfTwoModulo)
58+
if (value === null || typeof value !== "object") return false
59+
const e = value as Expr
60+
if (e.k === "arith" && e.op === "mod") {
61+
const d = foldConstant(e.r)
62+
if (d !== undefined && Number.isInteger(d) && d <= -2 && Number.isInteger(Math.log2(-d))) return true
63+
}
64+
return Object.values(value).some(negativePowerOfTwoModulo)
65+
}
66+
67+
const planIssues: ReadonlyArray<KnownIssue<PlanInput>> = [{
68+
id: "clickhouse-26.2-negative-power-of-two-modulo",
69+
// Server bug, not the builder's: on 26.2.19.43 a column % a constant -2^k is wrong for a negative
70+
// dividend — `SELECT (toInt64(number) - 5) % -2 FROM numbers(1)` is 1, not -1, and `% (0 - 2)` the
71+
// same. `% -3`, `% 2` and a constant dividend are right; 26.8 is right throughout. The target does
72+
// not know its server version, so this skips every ClickHouse target: delete it once CI drops 26.2.
73+
reason: "ClickHouse 26.2 computes a column % a negative power-of-two constant wrongly",
74+
matches: (input) => input.executed && input.target.startsWith("clickhouse") && negativePowerOfTwoModulo(input.plan),
75+
}]
5376

5477
export const knownPlanIssue = (input: PlanInput): string | undefined => firstMatch(planIssues, input)
5578

@@ -60,20 +83,7 @@ export interface UnionInput {
6083
readonly plan: UnionPlan
6184
}
6285

63-
/** A `%` by a negative constant anywhere in the input's plans. */
64-
const negativeModulo = (value: unknown): boolean =>
65-
Array.isArray(value) ? value.some(negativeModulo)
66-
: value !== null && typeof value === "object"
67-
? ((value as { k?: string; op?: string; r?: unknown }).k === "arith" && (value as { op?: string }).op === "mod" && typeof (value as { r?: unknown }).r === "number" && (value as { r: number }).r < 0)
68-
|| Object.values(value).some(negativeModulo)
69-
: false
70-
71-
const unionIssues: ReadonlyArray<KnownIssue<UnionInput>> = [{
72-
id: "clickhouse-26.2-union-negative-modulo",
73-
// Server bug, not the builder's: on 26.2, `SELECT A % -4 FROM t UNION ALL SELECT 0 FROM t` with A = -5
74-
// returns 3, not -1. 26.8 returns -1. Delete this entry once CI no longer runs 26.2.
75-
reason: "ClickHouse 26.2 computes % by a negative divisor wrongly under UNION ALL",
76-
matches: (input) => input.target.startsWith("clickhouse") && negativeModulo(input.plan),
77-
}]
86+
// Each branch is also checked against the plan issues above.
87+
const unionIssues: ReadonlyArray<KnownIssue<UnionInput>> = []
7888

7989
export const knownUnionIssue = (input: UnionInput): string | undefined => firstMatch(unionIssues, input)

‎tests/fuzz/model.ts‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,29 @@ const constant = (expr: Expr): boolean => {
328328
}
329329
}
330330

331+
/**
332+
* The number a constant arithmetic expression folds to, as the SQL text parses
333+
* it (see {@link asParsed}), or `undefined` when it reads a column or divides
334+
* by zero. `/` is float division, as on ClickHouse.
335+
*/
336+
export const foldConstant = (e: Expr | number): number | undefined => {
337+
if (typeof e === "number") return e
338+
if (e.k === "num") return e.v
339+
if (e.k !== "arith") return undefined
340+
const p = parsed.has(e) ? e : asParsed(e)
341+
if (p.k !== "arith") return foldConstant(p)
342+
const l = foldConstant(p.l)
343+
const r = foldConstant(p.r)
344+
if (l === undefined || r === undefined) return undefined
345+
switch (p.op) {
346+
case "add": return l + r
347+
case "sub": return l - r
348+
case "mul": return l * r
349+
case "div": return r === 0 ? undefined : l / r
350+
case "mod": return r === 0 ? undefined : l % r
351+
}
352+
}
353+
331354
export const evaluate = (plan: Plan, sem: Semantics, fixture: Fixture = coreFixture): ModelResult => {
332355
const env: Env = { sem, plan, joined: plan.join !== undefined }
333356
// A database may evaluate any expression on any row — or fold a constant one at plan time — before
@@ -336,7 +359,13 @@ export const evaluate = (plan: Plan, sem: Semantics, fixture: Fixture = coreFixt
336359
// A constant expression never reads this row. Reuse a real fixture row so the evaluator
337360
// can check folded subexpressions even when the input fixture or its inner join is empty.
338361
const constantRow: Row = { o: coreFixture.orders[0]! }
339-
for (const e of all) if (constant(e)) evalExpr(env, constantRow, e)
362+
// Check the chains as the SQL parses them, not as the plan nests them: `Id + 1 % 0` holds the
363+
// constant `1 % 0` although no plan node is constant. ClickHouse also runs `%` on an empty
364+
// block, so `Id % 0` fails over no rows; a constant zero divisor is refused on any data.
365+
for (const e of all.flatMap((e) => ariths(e.k === "arith" ? asParsed(e) : e))) {
366+
if (constant(e)) evalExpr(env, constantRow, e)
367+
if (e.k === "arith" && (e.op === "div" || e.op === "mod") && foldConstant(e.r) === 0) skip("division by zero")
368+
}
340369
for (const row of sourceRows(env, fixture)) for (const e of all) evalExpr(env, row, e)
341370
// A filter pushed below the join reads every order, even one an inner join would drop.
342371
if (plan.join !== undefined) for (const o of fixture.orders) for (const e of all) evalExpr(env, { o }, e)

‎tests/fuzz/queries.fuzz.test.ts‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const canonical = (rows: ReadonlyArray<unknown>): Array<unknown> =>
3333
const sorted = (rows: Array<unknown>) => rows.map((r) => JSON.stringify(r)).sort()
3434

3535
const matchesModel = (dialect: FuzzDialect, target: FuzzTarget, input: Plan, data?: Fixture) => {
36-
const known = knownPlanIssue({ target: target.name, plan: input })
36+
const known = knownPlanIssue({ target: target.name, plan: input, executed: true })
3737
if (known !== undefined) skipKnown(known)
3838
return checkRows(target, input.orgId, evaluate(input, target.semantics, data), compilePlan(dialect, data === undefined ? dialect.context : fixtureContext(dialect, data), input), compilePlan(dialect, scopeContext(dialect), input), input.where)
3939
}
@@ -42,7 +42,7 @@ const unionMatchesModel = (dialect: FuzzDialect, target: FuzzTarget, input: Unio
4242
const knownUnion = knownUnionIssue({ target: target.name, plan: input })
4343
if (knownUnion !== undefined) skipKnown(knownUnion)
4444
for (const branch of input.branches) {
45-
const known = knownPlanIssue({ target: target.name, plan: branch })
45+
const known = knownPlanIssue({ target: target.name, plan: branch, executed: true })
4646
if (known !== undefined) skipKnown(known)
4747
}
4848
return checkRows(
@@ -111,14 +111,22 @@ describe("generated fixture queries", () => {
111111
["empty orders", { orders: [], customers: [] }, undefined],
112112
["empty inner join", { orders: [{ OrgId: "org_1", Id: 1, Customer: "acme", Amount: 0, Status: "paid", Note: null, Created: Date.UTC(2026, 0, 1) }], customers: [] }, "inner"],
113113
] as const) {
114-
it(`constant division is excluded over ${label} (${target.name})`, () => {
115-
const input: Plan = {
116-
orgId: "org_1", join, where: [], wrap: false, crossTenant: false,
117-
shape: { k: "rows", select: [["id", { k: "col", col: "Id" }], ["bad", { k: "arith", op: "div", l: { k: "num", v: 1 }, r: 0 }]], distinct: false, orderBy: [] },
118-
}
119-
expect(() => evaluate(input, target.semantics, data)).toThrow(Skip)
120-
expect(() => evaluate(input, target.semantics, data)).toThrow("division by zero")
121-
})
114+
for (const [what, bad] of [
115+
["1 / 0", { k: "arith", op: "div", l: { k: "num", v: 1 }, r: 0 }],
116+
// `(Id + 1) % 0` in the plan is `Id + 1 % 0` in the SQL: the constant `1 % 0` is only there once parsed.
117+
["Id + 1 % 0", { k: "arith", op: "mod", l: { k: "arith", op: "add", l: { k: "col", col: "Id" }, r: 1 }, r: { k: "num", v: 0 } }],
118+
// ClickHouse runs `%` on an empty block too, so a zero divisor fails with no rows to read.
119+
["Id % 0", { k: "arith", op: "mod", l: { k: "col", col: "Id" }, r: 0 }],
120+
] as const) {
121+
it(`constant division ${what} is excluded over ${label} (${target.name})`, () => {
122+
const input: Plan = {
123+
orgId: "org_1", join, where: [], wrap: false, crossTenant: false,
124+
shape: { k: "rows", select: [["id", { k: "col", col: "Id" }], ["bad", bad]], distinct: false, orderBy: [] },
125+
}
126+
expect(() => evaluate(input, target.semantics, data)).toThrow(Skip)
127+
expect(() => evaluate(input, target.semantics, data)).toThrow("division by zero")
128+
})
129+
}
122130
}
123131
it.skipIf(!target.available)(`empty generated tables (${target.name})`, () => matchesModel(dialect, target, {
124132
orgId: "org_1", where: [], shape: { k: "rows", select: [["id", { k: "col", col: "Id" }]], distinct: false, orderBy: [] }, wrap: false, crossTenant: false,

0 commit comments

Comments
 (0)