Repository navigation
[pull] main from MaterializeInc:main - #1304
Merged
Merged
Conversation
Bumps [insta](https://github.com/mitsuhiko/insta) from 1.47.2 to 1.48.0. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/mitsuhiko/insta/releases">insta's releases</a>.</em></p> <blockquote> <h2>1.48.0</h2> <h2>Release Notes</h2> <ul> <li>Add <code>strip_ansi_escape_codes</code> setting which removes ANSI escape sequences (color codes, cursor movement, etc.) from snapshot content before comparison. Requires the <code>filters</code> feature. <a href="https://redirect.github.com/mitsuhiko/insta/issues/899">#899</a> (<a href="https://github.com/pierluigilenoci"><code>@pierluigilenoci</code></a>)</li> <li>Add opt-in support for YAML literal blocks for multiline strings in snapshot metadata fields such as <code>description</code> and <code>expression</code>. Set <code>INSTA_YAML_BLOCK_STYLE=1</code> to enable. <a href="https://redirect.github.com/mitsuhiko/insta/issues/851">#851</a> (<a href="https://github.com/ivov"><code>@ivov</code></a>)</li> <li>Setting <code>CI=true</code> normally makes <code>cargo insta test</code> behave as though <code>--check</code> was passed. Explicit snapshot handling options such as <code>--accept</code> now take precedence over this environment variable, allowing users to override this behavior if they want to. <a href="https://redirect.github.com/mitsuhiko/insta/issues/924">#924</a></li> <li>Fix <code>cargo insta test --profile</code> being forwarded to nextest as the nextest profile instead of the cargo build profile; it now translates to <code>--cargo-profile</code> for the nextest runner. Add <code>--nextest-profile</code> to select the nextest profile. <a href="https://redirect.github.com/mitsuhiko/insta/issues/910">#910</a></li> <li>Fix <code>cargo insta pending-snapshots</code> printing unusable <code>\\?\</code>-prefixed paths on Windows. The <code>--snapshot</code> filter now also accepts partial paths: any trailing path suffix of the snapshot file matches, so a bare <code>--snapshot my_test.snap</code> works. <a href="https://redirect.github.com/mitsuhiko/insta/issues/904">#904</a></li> <li>Accepting a binary snapshot no longer fails with <code>os error 2</code> when its data file is missing (e.g. gitignored and not committed). <a href="https://redirect.github.com/mitsuhiko/insta/issues/914">#914</a></li> </ul> <h2>Install cargo-insta 1.48.0</h2> <h3>Install prebuilt binaries via shell script</h3> <pre lang="sh"><code>curl --proto '=https' --tlsv1.2 -LsSf https://github.com/mitsuhiko/insta/releases/download/1.48.0/cargo-insta-installer.sh | sh </code></pre> <h3>Install prebuilt binaries via powershell script</h3> <pre lang="sh"><code>powershell -ExecutionPolicy Bypass -c "irm https://github.com/mitsuhiko/insta/releases/download/1.48.0/cargo-insta-installer.ps1 | iex" </code></pre> <h2>Download cargo-insta 1.48.0</h2> <table> <thead> <tr> <th>File</th> <th>Platform</th> <th>Checksum</th> </tr> </thead> <tbody> <tr> <td><a href="https://github.com/mitsuhiko/insta/releases/download/1.48.0/cargo-insta-aarch64-apple-darwin.tar.xz">cargo-insta-aarch64-apple-darwin.tar.xz</a></td> <td>Apple Silicon macOS</td> <td><a href="https://github.com/mitsuhiko/insta/releases/download/1.48.0/cargo-insta-aarch64-apple-darwin.tar.xz.sha256">checksum</a></td> </tr> <tr> <td><a href="https://github.com/mitsuhiko/insta/releases/download/1.48.0/cargo-insta-x86_64-apple-darwin.tar.xz">cargo-insta-x86_64-apple-darwin.tar.xz</a></td> <td>Intel macOS</td> <td><a href="https://github.com/mitsuhiko/insta/releases/download/1.48.0/cargo-insta-x86_64-apple-darwin.tar.xz.sha256">checksum</a></td> </tr> <tr> <td><a href="https://github.com/mitsuhiko/insta/releases/download/1.48.0/cargo-insta-x86_64-pc-windows-msvc.zip">cargo-insta-x86_64-pc-windows-msvc.zip</a></td> <td>x64 Windows</td> <td><a href="https://github.com/mitsuhiko/insta/releases/download/1.48.0/cargo-insta-x86_64-pc-windows-msvc.zip.sha256">checksum</a></td> </tr> <tr> <td><a href="https://github.com/mitsuhiko/insta/releases/download/1.48.0/cargo-insta-x86_64-unknown-linux-gnu.tar.xz">cargo-insta-x86_64-unknown-linux-gnu.tar.xz</a></td> <td>x64 Linux</td> <td><a href="https://github.com/mitsuhiko/insta/releases/download/1.48.0/cargo-insta-x86_64-unknown-linux-gnu.tar.xz.sha256">checksum</a></td> </tr> <tr> <td><a href="https://github.com/mitsuhiko/insta/releases/download/1.48.0/cargo-insta-x86_64-unknown-linux-musl.tar.xz">cargo-insta-x86_64-unknown-linux-musl.tar.xz</a></td> <td>x64 MUSL Linux</td> <td><a href="https://github.com/mitsuhiko/insta/releases/download/1.48.0/cargo-insta-x86_64-unknown-linux-musl.tar.xz.sha256">checksum</a></td> </tr> </tbody> </table> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/mitsuhiko/insta/blob/master/CHANGELOG.md">insta's changelog</a>.</em></p> <blockquote> <h2>1.48.0</h2> <ul> <li>Add <code>strip_ansi_escape_codes</code> setting which removes ANSI escape sequences (color codes, cursor movement, etc.) from snapshot content before comparison. Requires the <code>filters</code> feature. <a href="https://redirect.github.com/mitsuhiko/insta/issues/899">#899</a> (<a href="https://github.com/pierluigilenoci"><code>@pierluigilenoci</code></a>)</li> <li>Add opt-in support for YAML literal blocks for multiline strings in snapshot metadata fields such as <code>description</code> and <code>expression</code>. Set <code>INSTA_YAML_BLOCK_STYLE=1</code> to enable. <a href="https://redirect.github.com/mitsuhiko/insta/issues/851">#851</a> (<a href="https://github.com/ivov"><code>@ivov</code></a>)</li> <li>Setting <code>CI=true</code> normally makes <code>cargo insta test</code> behave as though <code>--check</code> was passed. Explicit snapshot handling options such as <code>--accept</code> now take precedence over this environment variable, allowing users to override this behavior if they want to. <a href="https://redirect.github.com/mitsuhiko/insta/issues/924">#924</a></li> <li>Fix <code>cargo insta test --profile</code> being forwarded to nextest as the nextest profile instead of the cargo build profile; it now translates to <code>--cargo-profile</code> for the nextest runner. Add <code>--nextest-profile</code> to select the nextest profile. <a href="https://redirect.github.com/mitsuhiko/insta/issues/910">#910</a></li> <li>Fix <code>cargo insta pending-snapshots</code> printing unusable <code>\\?\</code>-prefixed paths on Windows. The <code>--snapshot</code> filter now also accepts partial paths: any trailing path suffix of the snapshot file matches, so a bare <code>--snapshot my_test.snap</code> works. <a href="https://redirect.github.com/mitsuhiko/insta/issues/904">#904</a></li> <li>Accepting a binary snapshot no longer fails with <code>os error 2</code> when its data file is missing (e.g. gitignored and not committed). <a href="https://redirect.github.com/mitsuhiko/insta/issues/914">#914</a></li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/mitsuhiko/insta/commit/7f23d2e4684a789a85c50503004fb38a23c9ea58"><code>7f23d2e</code></a> Release 1.48.0 (<a href="https://redirect.github.com/mitsuhiko/insta/issues/925">#925</a>)</li> <li><a href="https://github.com/mitsuhiko/insta/commit/ee9cae1eb49cfa80c3d335246b0cfbb6bd006027"><code>ee9cae1</code></a> Allow <code>CI=true</code> to be overridden by an explicitly passed <code>--accept</code> CLI flag ...</li> <li><a href="https://github.com/mitsuhiko/insta/commit/043cf8268e3be93ca684fbfbf3969ad03ad71046"><code>043cf82</code></a> fix: translate --profile to --cargo-profile for nextest (<a href="https://redirect.github.com/mitsuhiko/insta/issues/913">#913</a>)</li> <li><a href="https://github.com/mitsuhiko/insta/commit/9c77f137b160e2235a71770d370fc1b4e7b1f456"><code>9c77f13</code></a> test: cover deep-wildcard redaction through arrays (<a href="https://redirect.github.com/mitsuhiko/insta/issues/915">#915</a>)</li> <li><a href="https://github.com/mitsuhiko/insta/commit/362f432b2d33ca9cdc0fe298a62942b3e13082d2"><code>362f432</code></a> Fix <code>--snapshot</code> filter on Windows; allow partial paths (<a href="https://redirect.github.com/mitsuhiko/insta/issues/904">#904</a>)</li> <li><a href="https://github.com/mitsuhiko/insta/commit/a436836bb22bd1c857f374c8aea6064120680558"><code>a436836</code></a> fix: tolerate a missing binary snapshot data file (<a href="https://redirect.github.com/mitsuhiko/insta/issues/914">#914</a>)</li> <li><a href="https://github.com/mitsuhiko/insta/commit/bf5fcdf1646087b717039011ac9a27cff3b4fc02"><code>bf5fcdf</code></a> fix: regenerate Cargo.lock and guard it with --locked in CI (<a href="https://redirect.github.com/mitsuhiko/insta/issues/912">#912</a>)</li> <li><a href="https://github.com/mitsuhiko/insta/commit/a761a9c937406d8eeab45f23d98d5a2a12105aaf"><code>a761a9c</code></a> feat: Support YAML literal blocks for multiline strings (<a href="https://redirect.github.com/mitsuhiko/insta/issues/851">#851</a>)</li> <li><a href="https://github.com/mitsuhiko/insta/commit/f9633f3a1251a10b8a96243cb941c7834e91288d"><code>f9633f3</code></a> ci: pin <code>check-minver</code> to nightly-2026-04-25 (<a href="https://redirect.github.com/mitsuhiko/insta/issues/905">#905</a>)</li> <li><a href="https://github.com/mitsuhiko/insta/commit/c7b98b8d23846130ce769f86ec8c33b0f221d48e"><code>c7b98b8</code></a> feat: add strip_ansi_escape_codes setting (<a href="https://redirect.github.com/mitsuhiko/insta/issues/899">#899</a>)</li> <li>See full diff in <a href="https://github.com/mitsuhiko/insta/compare/1.47.2...1.48.0">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Nothing major
@peterdukelarsen I'm guessing this is an expected tradeoff: ``` NAME | TYPE | THIS | OTHER | UNIT | THRESHOLD | Regression? | 'THIS' is -------------------------------------------------------------------------------------------------------------------------------------------------------- MySqlInitialLoadMultiWorkerSampled | wallclock | 2.923 | 2.258 | s | 10% | !!YES!! | worse: 29.4% slower MySqlInitialLoadMultiWorkerSampled | memory_mz | 1512.500 | 1524.902 | MB | 60% | no | better: 0.8% less MySqlInitialLoadMultiWorkerSampled | memory_clusterd | 172.387 | 129.578 | MB | 60% | no | worse: 33.0% more ``` Seen in [Feature benchmark against merge base or 'latest' 5](https://buildkite.com/materialize/nightly/builds/18121#01a01736-1416-4bf6-987b-ca14ebd0a209)
Improvements to the mz-debug-ci skill from stumbles in real sessions: - Accept a Buildkite build URL as input, and add a section on listing a build's failed jobs directly, with the job-state semantics: `failed`/`timed_out` are the failure states, `broken` means never dispatched, `soft_failed` and retried-then-passed jobs count as passing. - Annotation gotchas: `context` is `<JOB_ID>-error` and can reference retried attempts absent from the default job listing; non-mzcompose steps (lint, Security advisories) produce no annotations and no ci-failures rows, so go straight to their job log. - Log handling: services.log vs the console log for mzcompose jobs, log tails can be red herrings, CR/ANSI normalization before grepping. - A Timeouts category, known-vs-new triage steps (check whether main already fixed it, Linear issue keys), and a note on outdated bk versions rejecting the documented artifacts flags. The Buildkite API behaviors documented here (job states, retries, annotation contexts, commit statuses) were verified against real builds. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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. Issues these sharpened oracles found, fixed separately: * #37971, persist * #37972, avro * #37973, sql-parser * #37974, expr * #37975, repr, proto decode * #37976, repr, regex * #37977, repr, strconv * #37978, pgrepr * #37993, sql 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 [CPU-137](https://linear.app/materializeinc/issue/CPU-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](https://linear.app/materializeinc/issue/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](https://linear.app/materializeinc/issue/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 [CPU-137](https://linear.app/materializeinc/issue/CPU-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*. * In `mfp_optimize` the carve-out is also load-bearing for a second mechanism, the one release qualification 1330, 1332, 1334 and 1335 failed on: memoization hoists a fallible subexpression shared under an `AND`/`OR` into a mapped column that `SafeMfpPlan::evaluate_inner` runs eagerly, surfacing an error the lazy operator absorbs. #37974 originally changed `optimize` to prevent that, and after [review](<#37974 (comment)>) it no longer does: `AND`/`OR` evaluation order is undefined ([STG-54](https://linear.app/materializeinc/issue/STG-54)), so the surfaced error is a legal outcome of the same plan, and the oracle rather than the product has to tolerate it. * Non-temporal mode therefore takes `mir_scalar_reduce`'s row-precise `absorbs_and_or_operand_error` rather than the coarse whole-MFP skip, so only the rows that actually absorb an operand error stop asserting. The coarse predicate fires on 2.2% of MFPs (30,576 of 1,400,000), and those are exactly the `AND`/`OR`-over-fallible shapes where optimize's memoization is busiest, so dropping them entirely would blind the value and filter arms where a real miscompile is most likely to sit. Temporal mode keeps the coarse skip: its reference side substitutes a literal for `mz_now()` and re-runs `optimize` per probed time, so "absorbs on this row" has no single answer there. Verified in both directions against the unfixed product: a hand-crafted input generating `((#2 * #2) <= (#2 * #2)) IS NULL OR #4` with an overflowing `#2` crashes the current main oracle with exactly the release-qualification assertion and passes the row-precise one, and a twin input hiding the shared fallible part in an untaken `If` branch still crashes it when `eager_children`'s `If` guard is deliberately removed. * `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. `mfp_optimize`'s non-temporal mode adopts the same row-precise variant. The relationship between the coarse and row-precise forms 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 [CPU-137](https://linear.app/materializeinc/issue/CPU-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>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )