Skip to content

Persist pushdown hardening - #38110

Open
def- wants to merge 11 commits into
pr-per-63from
persist-pushdown-hardening
Open

Persist pushdown hardening#38110
def- wants to merge 11 commits into
pr-per-63from
persist-pushdown-hardening

Conversation

@def-

@def- def- commented Aug 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Motivation

Filter pushdown lets Materialize skip persist parts whose statistics prove no row can match a query's filter. A wrong skip is a silently wrong result, and a stats shape the decoder does not expect is a replica panic. This PR hardens the pipeline at every layer, stats collection, stats-to-range conversion, the interpreter's monotonicity contracts, and the runtime audit, and adds the test coverage that found the bugs.

Description

  • Wrong results: missing err stats let the peek path skip parts holding errors, and legacy V0 stats bounds could be read back at the wrong type.
  • Replica panics on durable data: undecodable part stats during version skew now fail open to fetching. Malformed err stats shapes and counts, malformed V0 stats, and wrong-kind fixed-size stats cross-decoding are all handled rather than panicking.
  • Audit: error rows of an audited part are now audited too. Previously the exact undercounted-err-stats violation class passed silently.
  • Guardrails: assert the dense-desc assumption apply_demand relies on, and document the name-keyed-stats and diffs_sum invariants along with why range_lower's monotone claim survives its NULLs.
  • Tests: poison-value corpus, deterministic stats containment sweep (every type, pairs, every trim budget), wide-schema multi-part end-to-end audit proptests with mz_now bounds and error rows, interpreter monotonicity vocabulary grown to ~150 declared-monotone functions, a coverage-guided pushdown_soundness cargo-fuzz target, and slt regressions for every SQL-reachable fix.
  • Fuzz runner: filtered runs build only the selected crates, signal-interrupted runs no longer report as crashes, and flags must precede --.

Verification

Every fix has a deterministic test verified red before the fix, unit or slt, except the audit error-row arm, where a test would require a live stats bug (explained in its commit message). Soaks: 16k-case interpreter equivalence runs over the full vocabulary, 131k-case wide e2e proptest, and a 160M-execution coverage-guided fuzz run, all clean. An analytic per-function proof pass of every is_monotone declaration backs the vocabulary.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Q5GiWioZM6CWrwWgfBADDR

def- and others added 11 commits August 7, 2026 10:58
@def- @claude
repr: check the kind tag when decoding fixed-size column stats
a04e3f4 
The Time, Timestamp, TimestampTz, Interval, and Uuid arms of col_values
matched any FixedSize stats. from_bytes validates length only, and
PackedNaiveDateTime, PackedInterval, and Uuid are all 16 bytes, so
wrong-kind bytes decoded silently into garbage bounds. Not reachable today
because a column's type cannot change under a reused name, but one schema
evolution feature away from wrong bounds on old parts. Mismatched kinds now
fall through to the catch-all arm and degrade to no stats.

Also soften the Timestamp arm's two hard expects on the roundtrip through
CheckedTimestamp to match the TimestampTz arm: malformed bytes should
degrade, not panic the replica.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5GiWioZM6CWrwWgfBADDR
@def- @claude
repr: validate V0 column stats instead of panicking on them
16740ac 
The legacy Atomic arm of col_values used four hard expects on ProtoDatum
decode: malformed V0 stats bytes panicked the replica at stats read time.
It also never validated the decoded datum against the column type, and the
V0 encoding carries no type tag, so a wrong-typed bound produced a range
that excludes every value of the column's actual type, i.e. wrong results
from corrupt legacy stats. Decode failures and type mismatches now degrade
to no stats via soft errors.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5GiWioZM6CWrwWgfBADDR
@def- @claude
storage-types: treat missing err stats as may-error in may_match_mfp
7857714 
may_match_mfp treated an absent err column in the part stats as "no
errors", while the storage read path's filter_result treats it as "may
error". The err column is on the default force-keep list for stats
trimming, but that list is configurable, so a trimmed part would have let
the peek path (StatsCursor uses may_match_mfp) skip a part whose error rows
must surface regardless of any filter. Align with filter_result and keep
the part.

The regression test builds a part with an error row, strips the err
column's stats, and applies a filter no Ok row matches: the old code
skipped the part.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5GiWioZM6CWrwWgfBADDR
@def- @claude
persist: fail open when part stats do not decode
cc79d0c 
The shard_source filter and the stats accessors in fetch called
LazyPartStats::decode, which panics on undecodable bytes, on stats that
other processes wrote. Stats from a newer version can use a proto variant
an older reader does not know, so a reader inside the upgrade window
panicked in the pushdown filter instead of falling back to fetching the
part, the fail-open behavior every other missing-stats case on this path
already has. Decode failures now degrade to "no stats".

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5GiWioZM6CWrwWgfBADDR
@def- @claude
storage-types: treat malformed err stats as unknown err count
bcf55e9 
ok_count had a hard expect on the err column's stats shape: corrupt or
version-skewed durable stats panicked the replica at pushdown time. An
unknown err count already fails open (callers keep the part), so degrade to
that instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5GiWioZM6CWrwWgfBADDR
@def- @claude
repr, persist: guard the latent filter pushdown invariants
494d9d3 
apply_demand filters metadata by raw ColumnIndex but types by position,
which only agree on dense descs. Assert that, so a future dropped-column
desc fails loudly instead of attaching statistics and filter specs to the
wrong columns. Document the two other invariants the residual-risk review
found load-bearing but unwritten: name-keyed part stats, and diffs_sum
substitution requiring registered batch bounds to match the blob.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5GiWioZM6CWrwWgfBADDR
@def- @claude
storage: audit error rows in wrongly-discarded parts
0b822c5 
The pushdown audit fired only when the MFP produced output on an Ok row.
Error rows from an audited part were emitted with no check, so a part
discarded because its err stats undercount, the one violation class the
err-count guard exists for, passed the audit silently. Mirror the audit
into the error arm. No deterministic test exists for this: the audit only
fires on an actual violation, which requires a live stats bug to construct
through the runtime operator.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5GiWioZM6CWrwWgfBADDR
@def- @claude
storage-types, expr, repr: systematic filter pushdown test coverage
66f424d 
Corpus: add -NaN and -0.0 floats, jsonb maps and lists, truncation-edge
strings, and a leap-second time to interesting_datums. Containment: check
that stats-derived specs contain every datum for every scalar type, over
datum pairs and full sets, after trim and trim_to_budget at every budget.
End to end: wide-schema and multi-part audit proptests over real part stats
with mz_now bounds and error rows, plus zero-column ReplaceWith and
schema-drift cases, and regression tests for both PER-53 variants in
pushdown.slt. Interpreter: grow the equivalence proptest vocabulary to 18
scalar types and ~150 declared-monotone functions, validating their
monotonicity claims continuously. PROPTEST_CASES now overrides the built-in
case counts for long runs, and a new coverage-guided pushdown_soundness
cargo-fuzz target explores raw bit patterns beyond the corpus with the same
soundness oracle.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5GiWioZM6CWrwWgfBADDR
@def- @claude
expr: document why range_lower's monotone claim survives its NULLs
895d19b 
range_lower maps empty and unbounded-lower ranges to NULL, which the
interpreter's endpoint box cannot represent. The claim is sound only
because those inputs form a downward-closed prefix of the range ordering,
so valued endpoints imply no NULL-yielding interior. Record that argument
at the declaration and pin the SQL results, verified empirically (range
columns also collect no statistics today, so pushdown cannot prune on
them).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5GiWioZM6CWrwWgfBADDR
@def- @claude
storage-types: don't underflow err_count on corrupt ok counts
32e48e3 
err_count subtracted the ok count from the part length unchecked: err
stats whose none count exceeds the part length (corrupt or version-skewed
durable state, the same class the previous hardening covers) underflowed
and panicked the replica. Report the err count as unknown instead, which
callers treat as may-error.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5GiWioZM6CWrwWgfBADDR
@def- @claude
expr, cargo-fuzz: address review nits
2cf0894 
Drop the duplicated value_between doc summary line, and document that the
fuzz runner's interrupted classification treats a kernel OOM SIGKILL as
interrupted too, relying on libFuzzer's rss limit to catch memory blowups
as artifact-producing OOMs first.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q5GiWioZM6CWrwWgfBADDR
@def- def- added the ci-no-test PR CI control: skip all tests (can't merge) label Aug 7, 2026
This was referenced Aug 7, 2026
Closed
Closed
Closed
@def-
def- marked this pull request as ready for review August 7, 2026 11:10
@def-
def- requested review from a team as code owners August 7, 2026 11:10
@def-
def- requested a review from DAlperin August 7, 2026 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DAlperin DAlperin Awaiting requested review from DAlperin

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

ci-no-test PR CI control: skip all tests (can't merge)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@def-