Skip to content

Persist pushdown hardening - #38096

Closed
def- wants to merge 11 commits into
MaterializeInc:pr-per-63from
def-:persist-pushdown-hardening
Closed

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

Conversation

@def-

@def- def- commented Aug 6, 2026 •
edited by github-actions Bot
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. After PER-53 showed the stats pipeline could derive impossible ranges from real data, this PR systematically 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.

User-visible changes

  • TIMESTAMP '2015-06-30 23:59:60' (a leap second) now rolls over to 2015-07-01 00:00:00, matching PostgreSQL, for timestamp and timestamptz literals/casts and for DATE + TIME. Previously it produced a value that sorted before midnight but extracted the same epoch. A leap second on the maximum supported date now errors as out of range.
  • Several wrong-results and panic bugs in filter pushdown are fixed (details below). Queries that previously returned too few rows on parts containing -NaN floats, or crashed replicas on undecodable/corrupt part statistics, now behave correctly.

Description

Product fixes, one commit each (squash-merge note: this PR spans persist-client, repr, expr, storage-types/operators, and test infra; the commit structure is intentional for review; happy to split into the stack (a) fail-open fixes (b) PER-53 wrong-results fixes (c) leap-second rollover (d) test framework (e) fuzz-runner changes if preferred):

  • Wrong results: mixed-sign NaNs defeating the PER-53 widening (float_bounds); missing err stats letting the peek path skip error parts; wrong-typed legacy V0 stats bounds.
  • Replica panics on durable data: undecodable part stats during version skew (fail open to fetching); malformed/corrupt err stats shapes and counts; malformed V0 stats; wrong-kind fixed-size stats silently cross-decoding.
  • 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; document the name-keyed-stats and diffs_sum invariants; document why range_lower's monotone claim survives its NULLs.
  • Leap seconds: roll :60 into the next minute at parse and in DATE + TIME, closing an analytic monotonicity-violation class (~8 declared-monotone functions).
  • Tests: poison-value corpus, deterministic stats containment sweep (every type x pairs x 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, slt regressions for every SQL-reachable fix.
  • Fuzz runner: filtered runs build only the selected crates; signal-interrupted runs no longer report as crashes; flags must precede --.

The first commit mirrors #38076 (PER-53) verbatim so this branch tests standalone; drop it on rebase if that lands first.

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, 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- def- added the ci-no-test PR CI control: skip all tests (can't merge) label Aug 6, 2026
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- changed the base branch from main to pr-per-63 August 7, 2026 11:00
@def-
def- force-pushed the persist-pushdown-hardening branch from eedb274 to 2cf0894 Compare August 7, 2026 11:00
@def- def- closed this Aug 7, 2026
@def-

def- commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #38110 (same 11 commits, head branch in this repo so it can take part in a GitHub stack). The PER-53 and leap-second commits now live in #38108 and #38109 below it. Top of stack #38111.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

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-