Add SQL fuzzing (sql_exec, sql_gen + TLP) and fix the bugs it found - #387
Merged
Merged
Conversation
Add a standalone cargo-fuzz crate under fuzz/ with an end-to-end SQL target that runs tests/slt scripts (slt syntax stripped) against an in-memory database. Any panic or sanitizer report fails the run. `make fuzz` runs it on a pinned nightly (nightly-2026-10-04) and CI runs it for 300s. Timeouts and OOMs are ignored for now via libFuzzer fork mode, since mutated queries can be legitimately unbounded (TODO noted).
Run libFuzzer with -ignore_ooms=0 so OOMs fail `make fuzz` like crashes; only timeouts stay ignored (TODO kept). Fork mode exits with the last child's exit code, so a run whose final job hit an ignored timeout exited 70 and failed. Build with `cargo fuzz build` and run the binary directly, treating 0 and 70 as success.
Tuple::deserialize_from_into pushed a NULL for every NULL column, including columns skipped by the projection (SkipFixed / SkipVariable), which yield no value otherwise. Later projected columns were shifted by one, so a plain `select c, b from t` could return wrong values, and type-specialized evaluators were handed values of the wrong type (UB in release builds).
Simplify moved arithmetic across comparisons with wrong formulas (e.g. `2 + b = 4` became `b = 6`), inverted `*` / `/` without regard to sign or integer truncation, mixed up terms of different columns, and typed rewritten nodes inconsistently (unreachable_unchecked). Replace it with Simplify::isolate_column, which only peels constant `+`/`-` terms in integer domains and unary signs, aborting on overflow. Negating the minimum integer panicked (debug) or wrapped (release); integer unary minus now returns DatabaseError::OverFlow like binary `+`/`-`, so UnaryEvaluatorRef::unary_eval returns a Result.
They declared a VARCHAR return type while producing an unsigned integer, so e.g. avg(char_length(c)) built a non-numeric accumulator, and they returned 0 instead of NULL for a NULL argument.
The select list shares its expression nodes with the DISTINCT aggregate's group keys. Rewriting a named alias in place turned e.g. the group key of `select distinct lower(c) as x, c` into a reference to the aggregate's own output. Rewrite a copy instead.
SumAccumulator asserted a numeric type, but sum(NULL) binds with the NULL literal's type. It never sees a non-NULL value, so the result is NULL.
Simplify expanded `e IN (a, b)` / `e BETWEEN a AND b` into comparisons that all pointed at the same `e` node. A later in-place position rewrite (e.g. predicate pushdown below a multi-way join) then shifted that node once per comparison, so it read the wrong column. Give each comparison its own copy.
…ries SELECT-list scalar subqueries are joined in at the Project step, above DISTINCT / GROUP BY / ORDER BY, so those clauses read another column instead of the subquery result: a crash when the types differ, a silently wrong result when they match (e.g. `select (select count(*) from t) - id as x from t order by x`). Reject such references until #386 is done.
ON / USING / NATURAL key pairs are split out of `l = r` at bind time, so they never got the cast `visit_binary` adds to a filter comparison. The join compares (and hashes) key values directly, so e.g. `on x.bigint_col = y.int_col` silently matched nothing. Cast both sides of each key pair to the common type when it is built.
CASE rejected branches of different types (e.g. `then bigint else int`) with Incomparable; unify them like a comparison does and cast each branch, as PostgreSQL does. Unary operators on the NULL literal (`- null`) were unsupported; they now yield NULL via a new evaluator position appended at the end.
Apply the remaining clippy suggestions (needless borrows, while-let loops,
byte strings, is_multiple_of, unit struct construction, ...) and remove
helpers only kept alive by tests: TableArena::expression,
PlanArena::{fill_parameters, alloc_dummy} and runtime_probe_depth.
The optimizer only checks a rule pattern's root predicate. The children predicates were only read by PlanMatcher, which nothing but its own tests used.
emit_tuple reported whether the joined tuple had any values, and a pair was only emitted when it did. With column pruning both sides can project no column at all (e.g. `count(*)` over a join), so every pair was dropped and the join returned no rows.
…L semantics IN compared values with DataValue's type-strict PartialEq and BETWEEN with partial_cmp, so e.g. `bigint_expr in (0, 2)` never matched and `double_col between 0 and 3` was always NULL; BETWEEN with a NULL bound also ignored three-valued logic. Like Binary, In and Between now carry `=` / `>=`, `<=` evaluators bound by BindEvaluator after unifying the operand types, so they share `=`'s semantics. Row value `=` / `<` treated NULL elements as equal (DataValue's PartialEq, which grouping relies on); they now follow SQL: `(4, NULL) = (4, NULL)` is NULL. Tuple types unify element-wise. crdb/where.slt is restored to the upstream expected result.
The pushed-down Limit copied OFFSET, so the outer side skipped those rows and the Limit above the join skipped them again. The rule also re-applied on every fix-point pass, stacking Limits; JoinOperator now records that a Limit was pushed.
`-9223372036854775808` was bound as unary minus over 9223372036854775808, which does not fit bigint and became a double, so inserting the bigint minimum overflowed. Fold `-<number>` into one literal as PostgreSQL does. crdb/overflow.slt is restored to the upstream bigint sum overflow test.
max_logical_type picked the longer row type, so e.g. `(0, 1, 2) = (0, 1)` was evaluated. PostgreSQL rejects it (unequal number of entries in row expressions); return Incomparable.
`*` hid the right USING column and output the left one, which is NULL for rows only in the right table. Named references already used coalesce(left, right); `*` now does the same. crdb/join.slt expects the query to succeed again instead of `statement error`.
and_or.slt expected errors for AND/OR short-circuit projections and select.slt made the `wide` columns NOT NULL so an insert failed; upstream expects both to succeed, and KiteSQL does.
Wildcard expansion of a table with a column alias list (e.g. `t AS a (x, y)`) took a separate path that still output the left USING column, which is NULL for rows only in the right table.
Wildcard expansion of a table with a column alias list iterated the alias map (a BTreeMap), so columns came out in alias-name order: `select * from t as l (lid, x, lv)` returned (lid, lv, x), and INSERT ... SELECT wrote values into the wrong columns. The loop was otherwise redundant with the schema loop below it, which already yields the alias columns in order; drop it.
`ORDER BY 2` was bound as the constant 2, so it did not sort at all. An integer literal now refers to that SELECT-list item, as in PostgreSQL; position 0 / out of range and non-integer constants are errors.
A bare column name present in several FROM sources resolved to the first one, and ORDER BY on an alias shared by different SELECT items used the last one. Both now fail with AmbiguousColumn, as in PostgreSQL.
`((SELECT ... ORDER BY a)) ORDER BY a` and a bare `SELECT *` were accepted; PostgreSQL rejects both.
It printed the column's arena slot, an internal value that depends on what else the process has bound.
sqllogictest 0.14 passed a `statement error` record whenever the statement returned rows, so those checks never ran; 0.29 checks them. All slt files now run on one runner and database, so each file drops what it creates and resets its sort mode. Two expectations that were wrong are fixed.
IN/ANY/ALL subqueries keep their projection, whose outer column refs were not rewritten to outer positions: they read the subquery's own row, so `id IN (SELECT b * 0 + t1.k FROM t2)` returned wrong rows, and column pruning panicked on them (found by sql_exec fuzzing). Reject such projections instead of returning wrong results.
`i32::MIN % -1` panicked (debug) on Rust's remainder overflow; PostgreSQL returns 0, which is what wrapping_rem gives. Found by sql_gen fuzzing.
sql_gen turns the fuzz input into well-typed queries over a fixed schema and checks them with ternary logic partitioning: the query without WHERE and its p / NOT p / p IS NULL split must return the same rows. Both fuzz targets now run on one LMDB database per process; `make fuzz` runs either target and CI smoke-runs both.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
make fuzzand smoke-run in CI:sql_exec: mutatestests/sltscripts; fails on panics, sanitizer reports and OOMs.sql_gen: generates well-typed queries over a fixed schema and checks their results with a TLP oracle.statement erroris actually checked; slt files now share one database and clean up after themselves.Testing
cargo test --lib,make test-slt, clippy, fmtsql_gen300s andsql_exec120s locally, no failuresNot covered
sql_execignores timeouts, since a running statement cannot be interrupted yet.