tests: sharpen the cargo-fuzz oracles across the workspace - #37979
Draft
def- wants to merge 2 commits into
Draft
Conversation
Everything here is a fuzz target, its corpus, dictionary or seeding script, or
the runner. The edits to non-fuzz sources are all test-only or test-enabling:
a `#[derive(Debug, Eq, PartialEq)]` on `SourceExportStatementDetails` so its
round-trip oracle can compare and report, unit tests added inside
`mz_avro::schema`'s `mod tests`, a proptest added to `mz_repr::adt::interval`,
and a `pub use` in `mz_pgwire` exposing the auth sub-parsers that
`Codec::decode` never calls, so the pre-auth SASL/password grammars are
reachable from the codec fuzzer at all, and a note on `mz_sql`'s exhaustive
`OptimizerFeatures` destructuring pointing at the fuzz crate's `fuzz_features`
helper, and a `strconv::element_needs_escaping`
predicate exposing the union of the list, map and record escaper rules so that a
formatter's `Nestable` verdict can be checked against the real ones.
The recurring theme is oracles that could pass while the behaviour they name is
broken, generators that stall before the decode they exist to drive, and
coverage the module docs claimed but never reached.
Formatting and tooling:
* `bin/fmt` skipped the fuzz crates entirely. `cargo metadata` only reports the
workspace it is pointed at, and the `src/*/fuzz` crates attach to
`test/cargo-fuzz`, which the root workspace does not include. Run it over both
manifests, keyed by edition, since `gen` is an identifier in 2021 but a
reserved keyword in 2024 and rustfmt cannot parse a file at the wrong edition.
* Pin `style_edition = "2024"` in rustfmt.toml. Formatting style is repo-wide
and independent of the edition a crate parses as, but overriding `edition`
per package would otherwise drag the 2021 import-sorting style along with it.
Runner and profiles:
* Release qualification runs `--profile all`, not `--profile fruitful`. A target
nobody runs is a target whose oracle can quietly stop asserting anything, and
the 24h budget is the only place the low-yield targets get exercised at all.
The profile docs said the opposite.
* Pair the two durable-state decoders (rollup = full snapshot, state diff =
incremental) in `fruitful`. Both are read from blob/consensus on every state
load, so a decode panic there makes the shard unloadable.
* Add `strconv_parse_interval` to `fruitful`. Every other hand-written strconv
text parser is in the "untrusted text/bytes parsers" bucket, so what that
file's own doc calls "the most intricate parser in strconv" was the one
omission, and did not run under the 24h release-qualification profile at all.
* Record that ASan does not cover the cc-built C dependencies. The
justification for defaulting `--sanitizer=none` claimed our targets cannot
find memory-corruption bugs. Some can: `ProtoNumeric` decode hands untrusted
bytes to libdecnumber's `decPackedToNumber`, which writes past the coefficient
it is given. Keep the default, because ASan does not help there either.
cargo-fuzz instruments through RUSTFLAGS while the C is compiled by the `cc`
crate without them, so the FFI writes stay invisible unless CFLAGS carry
`-fsanitize=address` too. Say so, so the next reader does not read the old
wording as "no target can hit memory corruption".
mz-avro `schema_resolve`:
* Require the resolutions documented as infallible instead of guarding them.
Identity and writer->reader resolution succeed by construction, so
`if let Ok(..)` meant a promotion that regressed at a non-union position came
back as a top-level `Err` and switched the decode oracle off for exactly the
inputs that found it. Deleting `ResolveIntDouble` from reader.rs now fails
within seconds; before it left the target green.
* Draw the encoded union branch from the input. Decoding consults only the
resolved permutation entry for the branch on the wire, so the fixed
first-promotable choice left every other entry's deferred `Err` unobserved,
the exact failure mode this target exists for.
* Reach the other two union arms: the reader rendering now sometimes collapses a
union to the encoded branch (`ResolveUnionConcrete`) and sometimes wraps a
concrete node in one (`ResolveConcreteUnion`), whose no-match error path has a
fixed out-of-bounds history. Unions also carry named variants now, for the
name-keyed side of variant matching.
* Drop trailing reader enum symbols so the `default` becomes load-bearing, and
require each decode to consume every byte the writer wrote.
mz-avro `reader_decode`:
* Straddle `safe_len` deliberately. All three "corrupt framing" magnitudes sat
above `MAX_ALLOCATION_BYTES`, so every one of them stopped at that guard: the
short-read case was dead and duplicated the negative-size case, and the
accepted-but-oversized count band, where the bug this target is credited with
finding still lived, was never generated.
* Stop generating schemas the parser rejects outright (`fixed` size 0,
`["null","null"]`, nested unions), which discarded ~6% of inputs at the header
with no block decoding at all.
mz-avro `avro_schema_parse`:
* Reach `MAX_SCHEMA_DEPTH`. It was unreachable through `Schema::from_str`:
serde_json's own 128-container limit fires one level first, so the target's
headline claim of driving the guard past its limit was never true. The deep
case now goes through `Schema::parse`, and the text case is bounded to nesting
serde_json accepts. Adds the unit test the depth guard never had, against a
hand-built `Value`, plus one pinning why the text entry point cannot reach it.
* Generate two named types sharing a fullname, the only shape that reaches
`alloc_name`'s duplicate rejection.
* Correct the doc: two of the "must reject" shapes are accepted today
(`parse_record` has no duplicate-field-name check, and `parse_bytes` swallows
a decimal `scale > precision`), and alias collisions mean nothing until
`resolve_schemas`, which this target never calls.
* Seeds are prefixed with the byte that selects `reader_decode`'s raw-bytes
branch. `reader_decode` reads its input as an `Unstructured` recipe, so the
seeded container file was being consumed as generator choices and never
reached the magic-header check that prepare-corpus.sh exists to skip past.
Catalog serde round-trip:
* Route the oracle through the durable encoding. The on-disk form is not JSON
text: `StateUpdateKindJson` packs the serde value into a `Jsonb`, i.e. an
`mz_repr::Row`, and reads it back through `to_serde_json` + `from_value`. That
leg is the one that can lose information, since every number becomes a
`Datum::Numeric` and object keys are deduplicated and reordered by the `Row`
map encoding, and `to_vec`/`from_slice` never touches it. The text form is
still asserted alongside it.
* Add `StateUpdateKind`, the durable envelope every catalog write goes through
and the only member of this family whose round trip is not trivially total: it
is `#[serde(tag = "kind")]`, so it deserializes through serde's
content-buffering path. Every other reachable field bottoms out in integers,
bools, strings, `Vec`, `Option`, or fieldless enums, for which the derived
`Serialize`/`Deserialize` are inverses by construction.
* Drop the `json == json2` assertion. Serialization is a pure function of the
fields and every reachable type derives structural `PartialEq`, so it was
implied by the value equality above it. It earns its keep in the proto
siblings this was modelled on, where `from_rust` + `encode_to_vec` can differ
for equal values, but not here.
* Both arms now dispatch over one type list in the same order, so a corpus
entry's type byte survives a one-bit change to the arm byte. Seed the corpus
and add a dictionary: the raw arm feeds the input straight to
`from_slice::<T>`, which returns nothing until the input is a complete object
with the right field names and variant tags, so the nested types were
unreachable.
`build_regex`:
* The target capped patterns at 4096 chars on the premise that "the `regex`
crate is size-limited, so an oversized pattern returns an error rather than
OOMing". That premise is wrong. `size_limit` bounds only the compiled NFA. The
memory is spent earlier, translating the pattern's AST into `regex-syntax`'s
HIR, which `size_limit` never sees, and cost there scales with the pattern's
source length. Measured with regex 1.12.4 and the exact `RegexBuilder`
configuration `build_regex` uses, `\p{L}` repeated under `i`, peak RSS is
~7.4 KB per pattern byte against ~0.35 KB for a plain literal: 4 095 bytes ->
47 MB, 524 285 -> 3 903 MB, 1 048 575 -> 7 792 MB, all returning
`CompiledTooBig`. The cap sat 87x under the runner's `-rss_limit_mb=4096` and
was dead besides, since the runner passes no `-max_len` and libFuzzer's
default is 4096 bytes.
* Add a rare `gen_long_pattern` arm that synthesizes a long pattern by repeating
one small unit, so the length comes from a handful of input bytes rather than
from `-max_len`, and straddles the 1 MiB guard. Drop the pattern cap, keep the
text cap. The arm now reaches 4043 MB of peak RSS, and libFuzzer reports an
11-byte input that spends 59 s building a 513 845-byte pattern.
* Correct three doc claims: the module premise above, `gen_near_limit`'s claim
that its output can hit `PatternTooLarge` (those patterns are <= ~45 bytes, so
only `CompiledTooBig` is reachable), and its example, which put the innermost
quantifier inside the group. `REPLACEMENT`'s `$99` was documented as an
out-of-range capture index but parses as the capture *name* `99y`, since an
unbraced `$name` takes the longest run of `[0-9A-Za-z_]`, which took the
name-lookup path and silently swallowed the trailing literal. Brace it as
`${99}`.
`EvalError` proto round-trip:
* The round-trip assertion alone certified a decoded `EvalError` that aborted
the process the moment anything looked at it: `OutOfDomain(None, None, _)`
survives the wire intact, and `Display` panicked on exactly that state. Assert
that the decoded value also renders, which is what `DataflowErrorSer::Display`
does with untrusted bytes on the index peek path. Verified falsifiable both
ways: a 15-byte mode B input crafted to decode as
`OutOfDomain(None, None, "x")` crashes the target against the pre-fix
`Display` and passes after it, and 1.3M executions are clean on the fixed
tree.
* Handle a `new_tree` rejection by returning rather than panicking, matching the
sibling round-trip targets, so a generator limit cannot be reported as a
product crash. Correct the module doc: `EvalError` carries no
`NumericMaxScale`, and it is mode B, not mode A, that reaches the rejection
branches of `char::from_proto` and `usize::from_proto`, since mode A always
starts from an already-valid Rust value.
Transform equivalence oracles:
* Add `hits_non_strict_error_fold`, which skips plans matching the open bug
CLU-137 rather than rediscovering it on every run. `Or::eval` returns `true`
the moment it sees a true operand and drops any error it collected,
`And::eval` does the same for `false`, and `error_if_null` evaluates its
message operand only when the first operand is NULL, yet `reduce`'s generic
variadic fold replaces the whole call with an operand's literal error and
`undistribute_and_or` can recombine an erroring operand across the
short-circuit boundary. Deliberately conservative: it asks whether an operand
*could* error, because `reduce` folds a column-free fallible operand to a
literal error first and absorbs it after. Measured at 3.6% of
`gen_rel(depth = 4)` plans skipped.
`interchange::avro::Decoder` over a fuzzer-generated reader schema:
* Gate every "don't build a valid body" branch on remaining input.
`Unstructured::int_in_range` does not fail once the data is exhausted, it
returns the low end of the range forever, and schema generation consumes
input greedily, so the ungated `== 0` branches were taken by every exhausted
input. Measured over 400k executions with the arms instrumented: empty bodies
fell from 36.5% to 4.3% of schema-directed runs, the clean-body oracle armed
on 5.8% before and 13.6% after, and the same budget found 1035 new coverage
units against 608, at a 7% throughput cost.
* Make the corruption arms corrupt. `truncate(keep)` drew `keep` from
`0..=b.len()`, so a `keep == b.len()` draw was a no-op, and the byte flip used
`^= arbitrary::<u8>()`, so a zero mask was one too.
* Stop generating schemas mz-avro rejects, which discarded the whole input:
`fixed` size 0 (`parse_fixed` requires a positive size) and
`["null",["null",T]]` (`UnionSchema::new` rejects a union directly inside a
union). Schema rejection went from 3.4% of schema-directed runs to zero.
* Bound a `decimal`-over-`fixed`'s precision by the chosen byte run. Precision
and size were drawn independently, and `parse_fixed` silently demotes an
over-precision decimal to a plain `fixed` with a `warn!` rather than an error,
so 10.2% of generated `DecimalFixed` types never reached the decimal path
while the harness still believed they had.
* Assert a clean body is consumed in full. A length-accounting bug that
under-consumes while still packing a well-formed `Row` passed every other
oracle.
* Correct two coverage claims. The `assert_success` oracle cannot reach the
deferred union-promotion class it cited (#37087): with
`WriterSchemaProvider::None` the resolver returns the reader schema verbatim,
so no `SchemaPiece::Resolve*` piece is ever built, and that class is covered
by `avro/fuzz::schema_resolve` instead. And the target never runs
`validate_schema_1`/`validate_schema_2`/`get_union_columns`, which are
reachable only from `schema_to_relationdesc`, so what it exercises is the
runtime `AvroFlatDecoder::union_branch`, not the column-name invention.
`mz_repr::adt::interval` proto round-trip:
* Replace the `interval_proto_roundtrip` fuzz target with a proptest next to the
type. `Interval` <-> `ProtoInterval` is a field-for-field copy of three
integers, so there is no untrusted-input surface for a fuzzer to explore: the
property only bites once the two structs drift, which a proptest over
`any::<Interval>()` catches just as well and runs in CI on every commit.
`strconv` time and timestamp round-trips:
* `strconv_parse_time` skipped every parsed value whose nanosecond field was not
an exact multiple of 1000, then asserted exact equality across
`format_time`/`parse_time`. Post-filter that pair is a bijection, so neither
the assertion nor the re-parse-failure return could ever fire and the
effective property was only "`parse_time` must not panic". The skipped region
was also the only part of the space where the renderer does anything
interesting, so `format_time` never saw a sub-microsecond fraction or a leap
second at all. Render unconditionally and assert bounded drift instead: the
renderer writes microseconds rounding half away from zero, so a faithful
rendering moves the value by at most half a microsecond. Measure the drift as
nanoseconds from midnight rather than field by field, because a fraction that
rounds up legitimately carries into the seconds field and on past a minute or
hour boundary. The one looser case is a carry out of the last second of the
day, where there is no second to carry into and the fraction saturates at
`.999999`. Falsifiable: dropping the renderer back to truncation is reported
in under a minute (`00:08:00.000000800` renders as `00:08:00`).
* Both timestamp targets fixed the rounding precision at `None`, i.e.
microseconds, which makes `round_to_precision`'s rounding branch dead: the
quantum is a single microsecond, so every value is already on a boundary and
only the seventh-digit nudge runs. The cast they mirror carries an arbitrary
`Option<TimestampPrecision>` from the column's type modifier, and a precision
below 6 is where rounding up off the end of `HIGH_DATE` is easy to reach, a
single `.5` at precision 0 rather than seven exact digits. Take the precision
from the input.
* Add a second oracle for the consumer that does not round.
`mz_pgrepr::Value::decode_text`, behind COPY and text-format bind parameters,
stores the parsed nanoseconds as they are, so for it the renderer is itself
the rounding step, and rendering plus re-parsing must land exactly on the
microsecond-rounded value. The carve-out is where rounding up leaves chrono's
range: the renderer has no error channel and saturates the fraction instead,
so there is nothing for the two to agree on.
* Drop both escape hatches. A parsed `:60` now round-trips, so the leap-second
carve-out's comment no longer holds. The re-parse-failure return's stated
reason, a year of more than four digits, was simply wrong:
`262142-12-31 23:59:59` parses and round-trips byte-identically, and over
395,498 successful parses the guard fired zero times. A renderer that emits
text the parser rejects is a bug to report, not tolerate, so both are now
`expect`.
* Add `strconv_parse_timestamp.dict` and `strconv_parse_timestamptz.dict`.
Neither end of the accepted range is reachable by mutation alone: `HIGH_DATE`
is the single date `262142-12-31`, so hitting it means mutating a 6-digit
ASCII integer onto one exact value while a late time-of-day and a westward
offset are already in place, and the range check offers no coverage gradient
to climb, just one saturated edge for "accepted" and one for "rejected".
`src/repr/fuzz` shipped no dictionary at all, and its corpus directory is
gitignored, so there were no seeds either. With the boundary dates, the
round-up fractions and the sub-minute offsets spelled out as tokens, an
empty-corpus run reproduces the timestamptz rounding overflow in under three
minutes, on `(precision 2, "262142-12-31 ?23:59:59.9999995")`.
* Verified by A/B: against the pre-fix renderer the unrounded oracle fails on
`2020-01-01 00:00:00.9999999` with `00:00:01` vs `00:00:00.100`, and on a leap
second because the rendering no longer parses. Against the fixes, three
minutes each of `strconv_parse_timestamp` (7.1M execs),
`strconv_parse_timestamptz` (7.2M) and `strconv_parse_time` (3.6M) are clean.
`strconv_parse_interval`:
* Assert the round trip the module doc names. The oracle returned silently when
the rendering failed to re-parse, so only the "if it re-parses, values agree"
half was checked and any rendering the parser rejects was a silent success. The
justification given for that carve-out does not exist: the hours field is the
only unbounded one and is derived as `(micros / 1_000_000).abs() / 3600`, so
multiplying it back out during the re-parse cannot exceed `|micros|` and cannot
overflow. `i64::MAX` micros renders as `2562047788:00:54.775807` and re-parses
exactly, as does `i64::MIN`. A direct sweep of the `(months, days, micros)`
space, 300,776 triples over both extremes, an edge grid and a pseudo-random
fan-out, produced zero re-parse failures and zero mismatches, so re-parse is
total and the branch was dead.
* What the tolerance cost, measured by mutating `Display`'s months spelling to
one `parse_interval` rejects: the tolerant oracle ran 3,570,705 executions and
reported success, while the strict one reports the 4-byte input `1mon`
immediately. Against the real renderer 5.2M executions are clean.
* Render through `strconv::format_interval` rather than `Display` directly, since
that is the entry point `::text` and the pgwire encoders call. The two agree
today, `format_interval` being a `write!` of `Display`, so this only keeps the
target pointed at the right function if that stops being true. This matters for
a type whose rendering is load-bearing and has already changed once: the unit
spelling moved to `mon`/`mons` in v26.35 (SQL-463), filed as a wrong-result bug
because a client's parser stopped at the old spelling.
`strconv_parse_date`:
* Assert the `Nestable` verdict, not just the round trip. `format_date` returns a
`Nestable` alongside the rendering and the target dropped it, leaving half the
contract unchecked, and that half was wrong: the ` BC` suffix carries
whitespace, which every element escaper quotes for, yet `Nestable::Yes` was
returned unconditionally. A wrong `Yes` is invisible to a round-trip oracle,
because the wrongly-unquoted rendering still re-parses, so it needs an
assertion of its own. Checked against the real escapers through
`element_needs_escaping` rather than restating their rules in the target, where
a copy would go stale: SQL-466 already added a rule to that predicate.
* Make the re-parse a hard failure. The tolerance was borrowed from the timestamp
targets and neither reason applies to DATE. `format_date` writes no time
component, so the leap-second gap cannot appear in its output, and a year of
more than four digits parses, since `fill_pdt_date` reads a 6-digit leading
number followed by a dash as a full year. Sweeping the whole `Date` range,
exhaustively at both ends and across the BC/AD transition, leaves exactly four
reachable forms, a 4, 5 or 6 digit Common Era year and a 4 digit BC year, and
100,962 sampled dates produced zero re-parse failures. The branch was dead.
* Verified by A/B: with `format_date`'s verdict put back to an unconditional
`Nestable::Yes` the target reports `8==8=8= BC` within seconds, and against the
fix 7.9M executions are clean.
`sql_server_table_desc_proto_roundtrip`:
* Assert the constraint-string arm's oracle. The module doc and an inline comment
both stated it, "garbage strings must be rejected, valid ones must parse", and
neither half was checked: `Err` was discarded by an early `return`, and on `Ok`
nothing looked at which spelling produced it. The arm's whole input space is
`CONSTRAINT_TYPES.len() * 4` fixed cases, so without that assertion it
contributed nothing beyond "does not panic", and it was blind to all three ways
the parser can regress. A normalization or catch-all arm would start accepting
`primary key`; a valid spelling ceasing to parse would make every input take the
early return, leaving an inert arm indistinguishable from a passing one; and
swapping the two variants is preserved by a round trip. The variant is not
cosmetic, it drives `is_primary` on the `TableConstraint::Unique` that
purification writes into the generated subsource DDL, so misparsing a non-unique
upstream constraint as `PrimaryKey` declares a key that does not hold. Verified
by mutating the product parser three ways: trim/uppercase normalization,
`"PRIMARY KEY"` no longer parsing, and the two variants swapped. Each is now
reported within seconds, and the unmutated tree is clean.
* Give every arm its bytes from offset 1. Only the proptest arm consumes the
32-byte seed, yet all four skipped past it, so the other three read their
selector bytes from offset 33 and saw nothing but defaults until libFuzzer grew
inputs that far. That also meant the raw-bytes arm, the one meant to consume a
real encoded descriptor, decapitated any genuine blob added to the corpus, which
`--corpus-sync` accumulates across runs. The same 3-minute budget now reaches
3594 edges and 18382 features against 3507 and 17215 before.
* Unlock two `parse_data_type` branches that were unreachable by construction, not
merely rare. `precision` and `scale` were drawn `% 39`, capping both at 38, so
the `precision > 39` rejection and the `NumericMaxScale::try_from` failure could
never fire, while the header claimed the type list exercises every branch of the
mapping. `% 45` reaches both, and `SqlServerColumnDesc::new` folds either
rejection into an `Unsupported` decode type that still round-trips.
* Pin `max_length` 4 and 8. `real`/`float`/`double precision` select `F32`/`F64`
by byte width, not by name or precision, so the two decode types the header
documents were reachable only by luck through the two random length branches.
The inline comments claimed precision selected them, which was never the rule.
* Drop the per-column leaf round-trip loop. The table's `into_proto`/`from_proto`
delegate per element and equality is structural, so the table-level assertion
already implies it. The loop just doubled the arm's conversion work.
* Give the constraint arm's descriptor the columns its constraint names, instead
of pairing a constraint over `c0..cn` with an empty column list. Harmless against
today's oracle, but a constraint/column consistency check in `from_proto` would
have been reported as a crash on the arm's own inputs.
Transform oracle helpers:
* Stop swallowing `TransformError`. `optimize` discarded the whole enum with
`.ok()`, documented as skipping the case where "the `Typecheck` pass rejects the
plan". That case cannot happen. `Typecheck::actually_perform_transform` returns
`Ok(())` on every path and routes a type error through
`type_error!(true, ..)` -> `soft_panic_or_log!`, as does `Fixpoint`
non-convergence, and soft assertions default to `cfg!(debug_assertions)`, which
cargo-fuzz enables. Confirmed by handing `optimize` a `Filter` with an `int4`
predicate in the built fuzz binary: it panics at `typecheck.rs:1672`, it does
not return `None`. So the comment sent anyone extending the generators looking
for a silent skip that does not exist, while a rejected plan actually lands as a
`crash-*` artifact pointing into `Optimizer::optimize`, which CI reports as an
optimizer bug.
* What could still reach `.ok()` was the opposite of benign: every remaining
`TransformError` over these plans is an optimizer invariant violation, a
`Let`/`Get` on an unbound local id, a `Let` whose type changed under it, an ANF
rebinding that lost an identifier, all of which reach users as `internal error`.
`NormalizeLets` runs on every input and `RelationCSE` in `fixpoint_logical_02`,
so the harness ran through this code constantly while being structurally unable
to report anything in it. Measured false-pass width by making
`NormalizeLets::actually_perform_transform` return
`Internal("injected invariant violation")` on every input: the old oracle
completed **367,428 executions and reported success**. It now panics on the
first one. `fold_to_multiset`'s `.ok()?` and `collapse`'s two
`is_err() -> StuckFixpoint` arms folded the same class into a skip and get the
same treatment; `StuckFixpoint` still covers genuine non-convergence.
* Record why the `EvalError` skip in `fold_to_multiset` is required rather than a
blind spot. `predicate_pushdown` knowingly pushes a predicate that can error
into a join input and manufactures an error the unoptimized plan never raises
(database-issues#6258), so one side folding to an error while the other yields
rows is accepted behaviour, measured at roughly 1 in 1,500 executions. Without
the reference the next reader is likely to "fix" it into an assertion.
* Document `gen_rel`'s arity contract. The doc spelled out the non-negativity
contract for the caller-supplied `leaf` but not that it must return at least one
column. A zero-arity leaf makes the `Project` arm's `int_in_range(1..=arity)`
an empty range and underflows `arity - 1`, and worse than either panic,
`join_scalars` drops an arity-0 single-row input *after* `join` computed the
equivalences' global column offsets over the full input list, so the
equivalences and the returned schema would silently point at the wrong columns.
Both in-tree leaves return 1-3 columns, so this is a contract note, not a fix.
Transform generators and the three transform targets:
* Build some nodes as raw variants instead of through `MirRelationExpr`'s smart
constructors, which normalize away exactly the shapes the structural fusions
collapse: `map` extends an existing `Map`'s scalar list, `negate` cancels an
enclosing `Negate`, `union` flattens its operands, `project` composes into an
existing `Project`, `filter` merges/sorts/dedups predicates. `MapFusion`,
`NegateFusion` and `UnionFusion` therefore could not fire on *any* input, so
their two assertions compared a plan against a clone of itself. Measured over
25,714 plans before: 0, 0 and 0 firings against FilterFusion's 16,526 (which
survives only through its `canonicalize_predicates` call) and ProjectFusion's
6,046 (through identity-projection removal). After: 939, 2,934 and 10,504,
with the other three unchanged or slightly up.
* Rebuild `wrap_preserving` from raw variants. Two of its four arms were
unconditional no-ops, `filter` dropping a literal-true predicate and
`negate().negate()` cancelling to the identity, which silently reduced the
structural-rewrite oracle to `fold(rel) == fold(rel)`. It now changes the plan
on every input (40,001 of 40,001) against 81.6% before.
* Apply each fusion in the traversal order its real driver uses. `fusion::union`
and `ProjectionExtraction` call `visit_mut_post`; only the
`Filter`/`Project`/`Map`/`Negate` fusions are pre-order. The comment claimed
all six matched their drivers.
* Report the input plan, not the folded one, in `FoldConstants`' shape assertion.
Every other call site passes the original; this one had moved it.
* Plan with production's `OptimizerFeatures`, not `Default::default()`, which is
all-`false` and is what no deployment runs. 14 of the 19 boolean optimizer flags
default to `true` in production, including the three `EquivalencePropagation`
reads. `enable_eq_classes_withholding_errors` is the pointed one: it exists to
stop equivalence propagation suppressing errors, and `gen_scalar` deliberately
seeds error literals, so the one feature built for this generator's input class
was the one turned off. `fuzz_features` names only the flags whose
production default is `true` and takes the rest from `Default`, so it cannot
break this crate's slow sanitizer build. The tripwire for a newly added flag
stays where it is cheap: `mz_sql`'s
`optimizer_features_no_enable_for_item_parsing` already destructures
`OptimizerFeatures` exhaustively, and now says to enable the flag in
`fuzz_features` too when it ships on, per the repo convention that a new flag is
ON in the test configuration.
* Cap the oracles' constant folding at `FOLD_ROW_LIMIT` rows. `FoldConstants`'
join arm materializes the full cross product before applying equivalences and
`limit: None` disables its only size check, so the harness could ask for a
`Vec<(Row, Diff)>` bounded only by the product of every leaf. Declining to fold
is already a benign skip on both oracles, and the cap sits well above the
largest product these generators were measured to reach.
* Optimize before deciding the baseline is comparable, in
`optimizer_symbolic_equiv`. A plan whose result is an evaluation error folds to
an `Err` constant, which `collapse` reports as a skip, and returning there meant
never calling the optimizer on it: `gen_scalar` spends one of three literal-leaf
choices on `EvalError::DivisionByZero`, so a deliberately generated class of
input never reached the code under test at all, not even for panic coverage.
* Share one `collapse` between the two equivalence targets.
`full_optimizer_equiv` folded the optimized side with a single `FoldConstants`
pass, which cannot fold through a `Let`, while its sibling already iterated
`FoldConstants` + `NormalizeLets` to a fixpoint "to collapse any `Let`s the
optimizer's CSE introduced". `RelationCSE` gets perfect candidates from the
`Union` arm cloning `inner` into both branches, and the skip landed precisely on
the executions where the post-collapse stages had run on real structure.
* Correct two doc claims. Neither target skips on an optimizer error any more, so
the `Typecheck`-rejection rationale is gone from both. And of the five
transforms `full_optimizer_equiv` claimed as its unique coverage, only
`NonNullRequirements` and `RedundantJoin` run before step 2's trailing constant
folding; `Demand`, `SemijoinIdempotence` and `ReduceElision` sit in
`fixpoint_logical_01`/`_02` and see a bare `Constant`. Nothing in the suite
covers those three against exact key facts, which the doc now says.
* Record that `Collapse::StuckFixpoint` conflates "folding could not evaluate it"
with "it folded to an `Err`", that only the second is a determined result, and
why the tempting `Ok -> Err` assertion is not available today
(`predicate_pushdown` manufactures such errors, database-issues#6258).
Carving out CLU-137, once, in one place:
* Give the shape a single definition,
`MirScalarExpr::could_hit_nonstrict_error_fold`, in `mz_expr` next to the fold it
describes. Four oracles now compare evaluation across `reduce` and each needs to
skip it; two had already drifted, since the transform helper counted
`ErrorIfNull` as non-strict and `mir_scalar_reduce`'s copy did not.
* Apply it to `mfp_optimize`, which had no carve-out at all. `optimize` reduces
every map expression and predicate, and that target generates `.and()`/`.or()`
over a fallible vocabulary, so all three of its assertion arms were exposed: a
folded error makes a row the raw MFP passed error out, the literal is typed
non-nullable so a nullability-dependent rewrite above it can change the projected
value, and a predicate folded that way drops the row. Its existing tolerance
covers none of those, because it only forgives an error on a row the raw MFP
*filtered*. Measured cost of the skip: 2.2% of MFPs (30,576 of 1,400,000).
* `mir_scalar_reduce` keeps its row-precise variant rather than adopting the coarse
predicate, because the shape is common in its generator and skipping every
expression carrying it would cost most of the comparisons. The relationship
between the two is now written down at both ends.
`mir_scalar_reduce`:
* Skip a row that absorbs a non-strict AND/OR operand error outright, instead of
merely allowing it to reduce to an error. The old carve-out was gated on the
reduced side also being an error, justified as "an absorbed error cannot
resurface as a value". The fuzzer falsified that after 2.2M executions:
`Or([Literal(Err(DivisionByZero)), Column(3)])` evaluates to `Ok(True)`, because
`Or::eval` returns true on its first true operand and drops the error it
collected, while reduce replaces the whole call with the literal error. From
that point the reduced expression is folding a value the original never
computed, and the rewrites downstream do not all propagate it: the reported case
turned `CastBoolToString(IsNull(If(..)))` from `Ok("true")` into a literal
`Ok("false")`.
* That is CLU-137, which PR #37299 did not fix, and the wrong value is downstream
of it rather than a second defect, verified by reducing the `Or` in isolation.
So the whole row goes, matching how the transform targets skip the same shape
via `hits_non_strict_error_fold`. The skip stays per-row rather than per-
expression, so every non-absorbing row of an affected expression is still
compared.
* Checked that this does not blunt the oracle: swapping `reduce_if`'s literal-
condition branches is still reported within seconds, and 4.8M executions are
clean against the unmutated tree.
Workspace-wide sweep of the remaining targets:
* The same three corrections applied across the rest of the suite: generators
that reach the decode instead of stalling at an early reject, oracles that
assert the property they name rather than a weaker one it implies, and module
docs pruned of coverage that was never reached. Touches the targets in
`repr` (row/arrow/codec round-trips, `strconv` parsers, `numeric_arith`,
`range_ops`, the type-modifier proto round-trips), `expr` (`jsonb_get`,
`jsonb_path`, `cast_string`, `like_pattern_compile` and `like_pattern_escape`,
`agg_decompose`, `mir_scalar_reduce`, `timezone_convert`, `mfp_optimize`),
`pgwire` (`codec_decode`, plus corpus dictionary and seeding), `pgcopy`,
`pgrepr`, `pgtz`, `storage` (the four upsert targets), `storage-types`,
`sql-parser`, `mysql-util`, `postgres-util`, `sql-server-util`,
`persist-client` and `interchange`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… oracle
The `sql_roundtrip` fuzz target's `normalize` stripped `Expr::Nested` but did
not canonicalize the sign of a numeric literal, so `- (2)` normalized to
`Op("-", Number("2"))`. The parser never produces that AST: a `-` directly in
front of a nonzero number folds into `Number("-2")`. `check_display` then
printed the normalized AST as `- 2`, reparsed it back to `Number("-2")`, and
reported structural drift. A false positive, not a printer bug.
Port the canonicalization the sibling `grammar` target has carried since both
targets were added: rewrite `Number("-n")` to a unary minus over the bare
number, on both sides of the comparison. The parser chooses between the two
forms by context (a leading `- 1` folds, `a + - 1` does not), so they have to
compare equal.
The rewrite keys off the leading `-` in the literal text, which only the
parser's own fold can put there, so `- -2` does not become `--2`. `- 0` keeps
its `Op` form on both sides (the parser only folds nonzero numbers), leaving
the signed-zero print path under the oracle.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Important
Merge this last. The sharpened oracles in this PR detect the bugs fixed by
the eight PRs below, so landing it first turns release qualification red until
they are all in.
cargo-fuzzruns only inci/release-qualification/pipeline.template.yml, not in PR CI, so nothinghere will fail to warn you. Held as a draft until the list is clear.
mapidentifier so option values round-trip #37993 sqlThe eight are independent of each other and of this PR textually, so they can
land in any order.
Everything here is a fuzz target, its corpus, dictionary or seeding script, or
the runner. The edits to non-fuzz sources are all test-only or test-enabling:
a
#[derive(Debug, Eq, PartialEq)]onSourceExportStatementDetailsso itsround-trip oracle can compare and report, unit tests added inside
mz_avro::schema'smod tests, a proptest added tomz_repr::adt::interval,and a
pub useinmz_pgwireexposing the auth sub-parsers thatCodec::decodenever calls, so the pre-auth SASL/password grammars arereachable from the codec fuzzer at all, and a note on
mz_sql's exhaustiveOptimizerFeaturesdestructuring pointing at the fuzz crate'sfuzz_featureshelper, and a
strconv::element_needs_escapingpredicate exposing the union of the list, map and record escaper rules so that a
formatter's
Nestableverdict can be checked against the real ones.The recurring theme is oracles that could pass while the behaviour they name is
broken, generators that stall before the decode they exist to drive, and
coverage the module docs claimed but never reached.
Formatting and tooling:
bin/fmtskipped the fuzz crates entirely.cargo metadataonly reports theworkspace it is pointed at, and the
src/*/fuzzcrates attach totest/cargo-fuzz, which the root workspace does not include. Run it over bothmanifests, keyed by edition, since
genis an identifier in 2021 but areserved keyword in 2024 and rustfmt cannot parse a file at the wrong edition.
style_edition = "2024"in rustfmt.toml. Formatting style is repo-wideand independent of the edition a crate parses as, but overriding
editionper package would otherwise drag the 2021 import-sorting style along with it.
Runner and profiles:
--profile all, not--profile fruitful. A targetnobody runs is a target whose oracle can quietly stop asserting anything, and
the 24h budget is the only place the low-yield targets get exercised at all.
The profile docs said the opposite.
incremental) in
fruitful. Both are read from blob/consensus on every stateload, so a decode panic there makes the shard unloadable.
strconv_parse_intervaltofruitful. Every other hand-written strconvtext parser is in the "untrusted text/bytes parsers" bucket, so what that
file's own doc calls "the most intricate parser in strconv" was the one
omission, and did not run under the 24h release-qualification profile at all.
justification for defaulting
--sanitizer=noneclaimed our targets cannotfind memory-corruption bugs. Some can:
ProtoNumericdecode hands untrustedbytes to libdecnumber's
decPackedToNumber, which writes past the coefficientit is given. Keep the default, because ASan does not help there either.
cargo-fuzz instruments through RUSTFLAGS while the C is compiled by the
cccrate without them, so the FFI writes stay invisible unless CFLAGS carry
-fsanitize=addresstoo. Say so, so the next reader does not read the oldwording as "no target can hit memory corruption".
mz-avro
schema_resolve:Identity and writer->reader resolution succeed by construction, so
if let Ok(..)meant a promotion that regressed at a non-union position cameback as a top-level
Errand switched the decode oracle off for exactly theinputs that found it. Deleting
ResolveIntDoublefrom reader.rs now failswithin seconds; before it left the target green.
resolved permutation entry for the branch on the wire, so the fixed
first-promotable choice left every other entry's deferred
Errunobserved,the exact failure mode this target exists for.
union to the encoded branch (
ResolveUnionConcrete) and sometimes wraps aconcrete node in one (
ResolveConcreteUnion), whose no-match error path has afixed out-of-bounds history. Unions also carry named variants now, for the
name-keyed side of variant matching.
defaultbecomes load-bearing, andrequire each decode to consume every byte the writer wrote.
mz-avro
reader_decode:safe_lendeliberately. All three "corrupt framing" magnitudes satabove
MAX_ALLOCATION_BYTES, so every one of them stopped at that guard: theshort-read case was dead and duplicated the negative-size case, and the
accepted-but-oversized count band, where the bug this target is credited with
finding still lived, was never generated.
fixedsize 0,["null","null"], nested unions), which discarded ~6% of inputs at the headerwith no block decoding at all.
mz-avro
avro_schema_parse:MAX_SCHEMA_DEPTH. It was unreachable throughSchema::from_str:serde_json's own 128-container limit fires one level first, so the target's
headline claim of driving the guard past its limit was never true. The deep
case now goes through
Schema::parse, and the text case is bounded to nestingserde_json accepts. Adds the unit test the depth guard never had, against a
hand-built
Value, plus one pinning why the text entry point cannot reach it.alloc_name's duplicate rejection.(
parse_recordhas no duplicate-field-name check, andparse_bytesswallowsa decimal
scale > precision), and alias collisions mean nothing untilresolve_schemas, which this target never calls.reader_decode's raw-bytesbranch.
reader_decodereads its input as anUnstructuredrecipe, so theseeded container file was being consumed as generator choices and never
reached the magic-header check that prepare-corpus.sh exists to skip past.
Catalog serde round-trip:
text:
StateUpdateKindJsonpacks the serde value into aJsonb, i.e. anmz_repr::Row, and reads it back throughto_serde_json+from_value. Thatleg is the one that can lose information, since every number becomes a
Datum::Numericand object keys are deduplicated and reordered by theRowmap encoding, and
to_vec/from_slicenever touches it. The text form isstill asserted alongside it.
StateUpdateKind, the durable envelope every catalog write goes throughand the only member of this family whose round trip is not trivially total: it
is
#[serde(tag = "kind")], so it deserializes through serde'scontent-buffering path. Every other reachable field bottoms out in integers,
bools, strings,
Vec,Option, or fieldless enums, for which the derivedSerialize/Deserializeare inverses by construction.json == json2assertion. Serialization is a pure function of thefields and every reachable type derives structural
PartialEq, so it wasimplied by the value equality above it. It earns its keep in the proto
siblings this was modelled on, where
from_rust+encode_to_veccan differfor equal values, but not here.
entry's type byte survives a one-bit change to the arm byte. Seed the corpus
and add a dictionary: the raw arm feeds the input straight to
from_slice::<T>, which returns nothing until the input is a complete objectwith the right field names and variant tags, so the nested types were
unreachable.
build_regex:regexcrate is size-limited, so an oversized pattern returns an error rather than
OOMing". That premise is wrong.
size_limitbounds only the compiled NFA. Thememory is spent earlier, translating the pattern's AST into
regex-syntax'sHIR, which
size_limitnever sees, and cost there scales with the pattern'ssource length. Measured with regex 1.12.4 and the exact
RegexBuilderconfiguration
build_regexuses,\p{L}repeated underi, peak RSS is~7.4 KB per pattern byte against ~0.35 KB for a plain literal: 4 095 bytes ->
47 MB, 524 285 -> 3 903 MB, 1 048 575 -> 7 792 MB, all returning
CompiledTooBig. The cap sat 87x under the runner's-rss_limit_mb=4096andwas dead besides, since the runner passes no
-max_lenand libFuzzer'sdefault is 4096 bytes.
gen_long_patternarm that synthesizes a long pattern by repeatingone small unit, so the length comes from a handful of input bytes rather than
from
-max_len, and straddles the 1 MiB guard. Drop the pattern cap, keep thetext cap. The arm now reaches 4043 MB of peak RSS, and libFuzzer reports an
11-byte input that spends 59 s building a 513 845-byte pattern.
gen_near_limit's claimthat its output can hit
PatternTooLarge(those patterns are <= ~45 bytes, soonly
CompiledTooBigis reachable), and its example, which put the innermostquantifier inside the group.
REPLACEMENT's$99was documented as anout-of-range capture index but parses as the capture name
99y, since anunbraced
$nametakes the longest run of[0-9A-Za-z_], which took thename-lookup path and silently swallowed the trailing literal. Brace it as
${99}.EvalErrorproto round-trip:EvalErrorthat abortedthe process the moment anything looked at it:
OutOfDomain(None, None, _)survives the wire intact, and
Displaypanicked on exactly that state. Assertthat the decoded value also renders, which is what
DataflowErrorSer::Displaydoes with untrusted bytes on the index peek path. Verified falsifiable both
ways: a 15-byte mode B input crafted to decode as
OutOfDomain(None, None, "x")crashes the target against the pre-fixDisplayand passes after it, and 1.3M executions are clean on the fixedtree.
new_treerejection by returning rather than panicking, matching thesibling round-trip targets, so a generator limit cannot be reported as a
product crash. Correct the module doc:
EvalErrorcarries noNumericMaxScale, and it is mode B, not mode A, that reaches the rejectionbranches of
char::from_protoandusize::from_proto, since mode A alwaysstarts from an already-valid Rust value.
Transform equivalence oracles:
hits_non_strict_error_fold, which skips plans matching the open bugCLU-137 rather than rediscovering it on every run.
Or::evalreturnstruethe moment it sees a true operand and drops any error it collected,
And::evaldoes the same forfalse, anderror_if_nullevaluates itsmessage operand only when the first operand is NULL, yet
reduce's genericvariadic fold replaces the whole call with an operand's literal error and
undistribute_and_orcan recombine an erroring operand across theshort-circuit boundary. Deliberately conservative: it asks whether an operand
could error, because
reducefolds a column-free fallible operand to aliteral error first and absorbs it after. Measured at 3.6% of
gen_rel(depth = 4)plans skipped.interchange::avro::Decoderover a fuzzer-generated reader schema:Unstructured::int_in_rangedoes not fail once the data is exhausted, itreturns the low end of the range forever, and schema generation consumes
input greedily, so the ungated
== 0branches were taken by every exhaustedinput. Measured over 400k executions with the arms instrumented: empty bodies
fell from 36.5% to 4.3% of schema-directed runs, the clean-body oracle armed
on 5.8% before and 13.6% after, and the same budget found 1035 new coverage
units against 608, at a 7% throughput cost.
truncate(keep)drewkeepfrom0..=b.len(), so akeep == b.len()draw was a no-op, and the byte flip used^= arbitrary::<u8>(), so a zero mask was one too.fixedsize 0 (parse_fixedrequires a positive size) and["null",["null",T]](UnionSchema::newrejects a union directly inside aunion). Schema rejection went from 3.4% of schema-directed runs to zero.
decimal-over-fixed's precision by the chosen byte run. Precisionand size were drawn independently, and
parse_fixedsilently demotes anover-precision decimal to a plain
fixedwith awarn!rather than an error,so 10.2% of generated
DecimalFixedtypes never reached the decimal pathwhile the harness still believed they had.
under-consumes while still packing a well-formed
Rowpassed every otheroracle.
assert_successoracle cannot reach thedeferred union-promotion class it cited (avro: support numeric promotion for nullable (union) columns #37087): with
WriterSchemaProvider::Nonethe resolver returns the reader schema verbatim,so no
SchemaPiece::Resolve*piece is ever built, and that class is coveredby
avro/fuzz::schema_resolveinstead. And the target never runsvalidate_schema_1/validate_schema_2/get_union_columns, which arereachable only from
schema_to_relationdesc, so what it exercises is theruntime
AvroFlatDecoder::union_branch, not the column-name invention.mz_repr::adt::intervalproto round-trip:interval_proto_roundtripfuzz target with a proptest next to thetype.
Interval<->ProtoIntervalis a field-for-field copy of threeintegers, so there is no untrusted-input surface for a fuzzer to explore: the
property only bites once the two structs drift, which a proptest over
any::<Interval>()catches just as well and runs in CI on every commit.strconvtime and timestamp round-trips:strconv_parse_timeskipped every parsed value whose nanosecond field was notan exact multiple of 1000, then asserted exact equality across
format_time/parse_time. Post-filter that pair is a bijection, so neitherthe assertion nor the re-parse-failure return could ever fire and the
effective property was only "
parse_timemust not panic". The skipped regionwas also the only part of the space where the renderer does anything
interesting, so
format_timenever saw a sub-microsecond fraction or a leapsecond at all. Render unconditionally and assert bounded drift instead: the
renderer writes microseconds rounding half away from zero, so a faithful
rendering moves the value by at most half a microsecond. Measure the drift as
nanoseconds from midnight rather than field by field, because a fraction that
rounds up legitimately carries into the seconds field and on past a minute or
hour boundary. The one looser case is a carry out of the last second of the
day, where there is no second to carry into and the fraction saturates at
.999999. Falsifiable: dropping the renderer back to truncation is reportedin under a minute (
00:08:00.000000800renders as00:08:00).None, i.e.microseconds, which makes
round_to_precision's rounding branch dead: thequantum is a single microsecond, so every value is already on a boundary and
only the seventh-digit nudge runs. The cast they mirror carries an arbitrary
Option<TimestampPrecision>from the column's type modifier, and a precisionbelow 6 is where rounding up off the end of
HIGH_DATEis easy to reach, asingle
.5at precision 0 rather than seven exact digits. Take the precisionfrom the input.
mz_pgrepr::Value::decode_text, behind COPY and text-format bind parameters,stores the parsed nanoseconds as they are, so for it the renderer is itself
the rounding step, and rendering plus re-parsing must land exactly on the
microsecond-rounded value. The carve-out is where rounding up leaves chrono's
range: the renderer has no error channel and saturates the fraction instead,
so there is nothing for the two to agree on.
:60now round-trips, so the leap-secondcarve-out's comment no longer holds. The re-parse-failure return's stated
reason, a year of more than four digits, was simply wrong:
262142-12-31 23:59:59parses and round-trips byte-identically, and over395,498 successful parses the guard fired zero times. A renderer that emits
text the parser rejects is a bug to report, not tolerate, so both are now
expect.strconv_parse_timestamp.dictandstrconv_parse_timestamptz.dict.Neither end of the accepted range is reachable by mutation alone:
HIGH_DATEis the single date
262142-12-31, so hitting it means mutating a 6-digitASCII integer onto one exact value while a late time-of-day and a westward
offset are already in place, and the range check offers no coverage gradient
to climb, just one saturated edge for "accepted" and one for "rejected".
src/repr/fuzzshipped no dictionary at all, and its corpus directory isgitignored, so there were no seeds either. With the boundary dates, the
round-up fractions and the sub-minute offsets spelled out as tokens, an
empty-corpus run reproduces the timestamptz rounding overflow in under three
minutes, on
(precision 2, "262142-12-31 ?23:59:59.9999995").2020-01-01 00:00:00.9999999with00:00:01vs00:00:00.100, and on a leapsecond because the rendering no longer parses. Against the fixes, three
minutes each of
strconv_parse_timestamp(7.1M execs),strconv_parse_timestamptz(7.2M) andstrconv_parse_time(3.6M) are clean.strconv_parse_interval:the rendering failed to re-parse, so only the "if it re-parses, values agree"
half was checked and any rendering the parser rejects was a silent success. The
justification given for that carve-out does not exist: the hours field is the
only unbounded one and is derived as
(micros / 1_000_000).abs() / 3600, somultiplying it back out during the re-parse cannot exceed
|micros|and cannotoverflow.
i64::MAXmicros renders as2562047788:00:54.775807and re-parsesexactly, as does
i64::MIN. A direct sweep of the(months, days, micros)space, 300,776 triples over both extremes, an edge grid and a pseudo-random
fan-out, produced zero re-parse failures and zero mismatches, so re-parse is
total and the branch was dead.
Display's months spelling toone
parse_intervalrejects: the tolerant oracle ran 3,570,705 executions andreported success, while the strict one reports the 4-byte input
1monimmediately. Against the real renderer 5.2M executions are clean.
strconv::format_intervalrather thanDisplaydirectly, sincethat is the entry point
::textand the pgwire encoders call. The two agreetoday,
format_intervalbeing awrite!ofDisplay, so this only keeps thetarget pointed at the right function if that stops being true. This matters for
a type whose rendering is load-bearing and has already changed once: the unit
spelling moved to
mon/monsin v26.35 (SQL-463), filed as a wrong-result bugbecause a client's parser stopped at the old spelling.
strconv_parse_date:Nestableverdict, not just the round trip.format_datereturns aNestablealongside the rendering and the target dropped it, leaving half thecontract unchecked, and that half was wrong: the
BCsuffix carrieswhitespace, which every element escaper quotes for, yet
Nestable::Yeswasreturned unconditionally. A wrong
Yesis invisible to a round-trip oracle,because the wrongly-unquoted rendering still re-parses, so it needs an
assertion of its own. Checked against the real escapers through
element_needs_escapingrather than restating their rules in the target, wherea copy would go stale: SQL-466 already added a rule to that predicate.
targets and neither reason applies to DATE.
format_datewrites no timecomponent, so the leap-second gap cannot appear in its output, and a year of
more than four digits parses, since
fill_pdt_datereads a 6-digit leadingnumber followed by a dash as a full year. Sweeping the whole
Daterange,exhaustively at both ends and across the BC/AD transition, leaves exactly four
reachable forms, a 4, 5 or 6 digit Common Era year and a 4 digit BC year, and
100,962 sampled dates produced zero re-parse failures. The branch was dead.
format_date's verdict put back to an unconditionalNestable::Yesthe target reports8==8=8= BCwithin seconds, and against thefix 7.9M executions are clean.
sql_server_table_desc_proto_roundtrip:both stated it, "garbage strings must be rejected, valid ones must parse", and
neither half was checked:
Errwas discarded by an earlyreturn, and onOknothing looked at which spelling produced it. The arm's whole input space is
CONSTRAINT_TYPES.len() * 4fixed cases, so without that assertion itcontributed nothing beyond "does not panic", and it was blind to all three ways
the parser can regress. A normalization or catch-all arm would start accepting
primary key; a valid spelling ceasing to parse would make every input take theearly return, leaving an inert arm indistinguishable from a passing one; and
swapping the two variants is preserved by a round trip. The variant is not
cosmetic, it drives
is_primaryon theTableConstraint::Uniquethatpurification writes into the generated subsource DDL, so misparsing a non-unique
upstream constraint as
PrimaryKeydeclares a key that does not hold. Verifiedby mutating the product parser three ways: trim/uppercase normalization,
"PRIMARY KEY"no longer parsing, and the two variants swapped. Each is nowreported within seconds, and the unmutated tree is clean.
32-byte seed, yet all four skipped past it, so the other three read their
selector bytes from offset 33 and saw nothing but defaults until libFuzzer grew
inputs that far. That also meant the raw-bytes arm, the one meant to consume a
real encoded descriptor, decapitated any genuine blob added to the corpus, which
--corpus-syncaccumulates across runs. The same 3-minute budget now reaches3594 edges and 18382 features against 3507 and 17215 before.
parse_data_typebranches that were unreachable by construction, notmerely rare.
precisionandscalewere drawn% 39, capping both at 38, sothe
precision > 39rejection and theNumericMaxScale::try_fromfailure couldnever fire, while the header claimed the type list exercises every branch of the
mapping.
% 45reaches both, andSqlServerColumnDesc::newfolds eitherrejection into an
Unsupporteddecode type that still round-trips.max_length4 and 8.real/float/double precisionselectF32/F64by byte width, not by name or precision, so the two decode types the header
documents were reachable only by luck through the two random length branches.
The inline comments claimed precision selected them, which was never the rule.
into_proto/from_protodelegate per element and equality is structural, so the table-level assertion
already implies it. The loop just doubled the arm's conversion work.
of pairing a constraint over
c0..cnwith an empty column list. Harmless againsttoday's oracle, but a constraint/column consistency check in
from_protowouldhave been reported as a crash on the arm's own inputs.
Transform oracle helpers:
TransformError.optimizediscarded the whole enum with.ok(), documented as skipping the case where "theTypecheckpass rejects theplan". That case cannot happen.
Typecheck::actually_perform_transformreturnsOk(())on every path and routes a type error throughtype_error!(true, ..)->soft_panic_or_log!, as doesFixpointnon-convergence, and soft assertions default to
cfg!(debug_assertions), whichcargo-fuzz enables. Confirmed by handing
optimizeaFilterwith anint4predicate in the built fuzz binary: it panics at
typecheck.rs:1672, it doesnot return
None. So the comment sent anyone extending the generators lookingfor a silent skip that does not exist, while a rejected plan actually lands as a
crash-*artifact pointing intoOptimizer::optimize, which CI reports as anoptimizer bug.
.ok()was the opposite of benign: every remainingTransformErrorover these plans is an optimizer invariant violation, aLet/Geton an unbound local id, aLetwhose type changed under it, an ANFrebinding that lost an identifier, all of which reach users as
internal error.NormalizeLetsruns on every input andRelationCSEinfixpoint_logical_02,so the harness ran through this code constantly while being structurally unable
to report anything in it. Measured false-pass width by making
NormalizeLets::actually_perform_transformreturnInternal("injected invariant violation")on every input: the old oraclecompleted 367,428 executions and reported success. It now panics on the
first one.
fold_to_multiset's.ok()?andcollapse's twois_err() -> StuckFixpointarms folded the same class into a skip and get thesame treatment;
StuckFixpointstill covers genuine non-convergence.EvalErrorskip infold_to_multisetis required rather than ablind spot.
predicate_pushdownknowingly pushes a predicate that can errorinto a join input and manufactures an error the unoptimized plan never raises
(database-issues#6258), so one side folding to an error while the other yields
rows is accepted behaviour, measured at roughly 1 in 1,500 executions. Without
the reference the next reader is likely to "fix" it into an assertion.
gen_rel's arity contract. The doc spelled out the non-negativitycontract for the caller-supplied
leafbut not that it must return at least onecolumn. A zero-arity leaf makes the
Projectarm'sint_in_range(1..=arity)an empty range and underflows
arity - 1, and worse than either panic,join_scalarsdrops an arity-0 single-row input afterjoincomputed theequivalences' global column offsets over the full input list, so the
equivalences and the returned schema would silently point at the wrong columns.
Both in-tree leaves return 1-3 columns, so this is a contract note, not a fix.
Transform generators and the three transform targets:
MirRelationExpr's smartconstructors, which normalize away exactly the shapes the structural fusions
collapse:
mapextends an existingMap's scalar list,negatecancels anenclosing
Negate,unionflattens its operands,projectcomposes into anexisting
Project,filtermerges/sorts/dedups predicates.MapFusion,NegateFusionandUnionFusiontherefore could not fire on any input, sotheir two assertions compared a plan against a clone of itself. Measured over
25,714 plans before: 0, 0 and 0 firings against FilterFusion's 16,526 (which
survives only through its
canonicalize_predicatescall) and ProjectFusion's6,046 (through identity-projection removal). After: 939, 2,934 and 10,504,
with the other three unchanged or slightly up.
wrap_preservingfrom raw variants. Two of its four arms wereunconditional no-ops,
filterdropping a literal-true predicate andnegate().negate()cancelling to the identity, which silently reduced thestructural-rewrite oracle to
fold(rel) == fold(rel). It now changes the planon every input (40,001 of 40,001) against 81.6% before.
fusion::unionand
ProjectionExtractioncallvisit_mut_post; only theFilter/Project/Map/Negatefusions are pre-order. The comment claimedall six matched their drivers.
FoldConstants' shape assertion.Every other call site passes the original; this one had moved it.
OptimizerFeatures, notDefault::default(), which isall-
falseand is what no deployment runs. 14 of the 19 boolean optimizer flagsdefault to
truein production, including the threeEquivalencePropagationreads.
enable_eq_classes_withholding_errorsis the pointed one: it exists tostop equivalence propagation suppressing errors, and
gen_scalardeliberatelyseeds error literals, so the one feature built for this generator's input class
was the one turned off.
fuzz_featuresnames only the flags whoseproduction default is
trueand takes the rest fromDefault, so it cannotbreak this crate's slow sanitizer build. The tripwire for a newly added flag
stays where it is cheap:
mz_sql'soptimizer_features_no_enable_for_item_parsingalready destructuresOptimizerFeaturesexhaustively, and now says to enable the flag infuzz_featurestoo when it ships on, per the repo convention that a new flag isON in the test configuration.
FOLD_ROW_LIMITrows.FoldConstants'join arm materializes the full cross product before applying equivalences and
limit: Nonedisables its only size check, so the harness could ask for aVec<(Row, Diff)>bounded only by the product of every leaf. Declining to foldis already a benign skip on both oracles, and the cap sits well above the
largest product these generators were measured to reach.
optimizer_symbolic_equiv. A plan whose result is an evaluation error folds toan
Errconstant, whichcollapsereports as a skip, and returning there meantnever calling the optimizer on it:
gen_scalarspends one of three literal-leafchoices on
EvalError::DivisionByZero, so a deliberately generated class ofinput never reached the code under test at all, not even for panic coverage.
collapsebetween the two equivalence targets.full_optimizer_equivfolded the optimized side with a singleFoldConstantspass, which cannot fold through a
Let, while its sibling already iteratedFoldConstants+NormalizeLetsto a fixpoint "to collapse anyLets theoptimizer's CSE introduced".
RelationCSEgets perfect candidates from theUnionarm cloninginnerinto both branches, and the skip landed precisely onthe executions where the post-collapse stages had run on real structure.
the
Typecheck-rejection rationale is gone from both. And of the fivetransforms
full_optimizer_equivclaimed as its unique coverage, onlyNonNullRequirementsandRedundantJoinrun before step 2's trailing constantfolding;
Demand,SemijoinIdempotenceandReduceElisionsit infixpoint_logical_01/_02and see a bareConstant. Nothing in the suitecovers those three against exact key facts, which the doc now says.
Collapse::StuckFixpointconflates "folding could not evaluate it"with "it folded to an
Err", that only the second is a determined result, andwhy the tempting
Ok -> Errassertion is not available today(
predicate_pushdownmanufactures such errors, database-issues#6258).Carving out CLU-137, once, in one place:
MirScalarExpr::could_hit_nonstrict_error_fold, inmz_exprnext to the fold itdescribes. Four oracles now compare evaluation across
reduceand each needs toskip it; two had already drifted, since the transform helper counted
ErrorIfNullas non-strict andmir_scalar_reduce's copy did not.mfp_optimize, which had no carve-out at all.optimizereducesevery map expression and predicate, and that target generates
.and()/.or()over a fallible vocabulary, so all three of its assertion arms were exposed: a
folded error makes a row the raw MFP passed error out, the literal is typed
non-nullable so a nullability-dependent rewrite above it can change the projected
value, and a predicate folded that way drops the row. Its existing tolerance
covers none of those, because it only forgives an error on a row the raw MFP
filtered. Measured cost of the skip: 2.2% of MFPs (30,576 of 1,400,000).
mir_scalar_reducekeeps its row-precise variant rather than adopting the coarsepredicate, because the shape is common in its generator and skipping every
expression carrying it would cost most of the comparisons. The relationship
between the two is now written down at both ends.
mir_scalar_reduce:merely allowing it to reduce to an error. The old carve-out was gated on the
reduced side also being an error, justified as "an absorbed error cannot
resurface as a value". The fuzzer falsified that after 2.2M executions:
Or([Literal(Err(DivisionByZero)), Column(3)])evaluates toOk(True), becauseOr::evalreturns true on its first true operand and drops the error itcollected, while reduce replaces the whole call with the literal error. From
that point the reduced expression is folding a value the original never
computed, and the rewrites downstream do not all propagate it: the reported case
turned
CastBoolToString(IsNull(If(..)))fromOk("true")into a literalOk("false").of it rather than a second defect, verified by reducing the
Orin isolation.So the whole row goes, matching how the transform targets skip the same shape
via
hits_non_strict_error_fold. The skip stays per-row rather than per-expression, so every non-absorbing row of an affected expression is still
compared.
reduce_if's literal-condition branches is still reported within seconds, and 4.8M executions are
clean against the unmutated tree.
Workspace-wide sweep of the remaining targets:
that reach the decode instead of stalling at an early reject, oracles that
assert the property they name rather than a weaker one it implies, and module
docs pruned of coverage that was never reached. Touches the targets in
repr(row/arrow/codec round-trips,strconvparsers,numeric_arith,range_ops, the type-modifier proto round-trips),expr(jsonb_get,jsonb_path,cast_string,like_pattern_compileandlike_pattern_escape,agg_decompose,mir_scalar_reduce,timezone_convert,mfp_optimize),pgwire(codec_decode, plus corpus dictionary and seeding),pgcopy,pgrepr,pgtz,storage(the four upsert targets),storage-types,sql-parser,mysql-util,postgres-util,sql-server-util,persist-clientandinterchange.