Skip to content

clusterd-test-driver: add explain verb to assert optimized plan shape - #37141

Merged
antiguru merged 1 commit into
MaterializeInc:mainfrom
antiguru:headless-driver-explain
Aug 26, 2026
Merged

clusterd-test-driver: add explain verb to assert optimized plan shape#37141
antiguru merged 1 commit into
MaterializeInc:mainfrom
antiguru:headless-driver-explain

Conversation

@antiguru

Copy link
Copy Markdown
Member

Follow-up to #37008 (now merged), addressing DAlperin's review on join.spec: a script using optimize asserts only the result, so optimizer or lowering drift could silently change the plan under test.

explain renders the lowered LIR plan (the EXPLAIN PHYSICAL PLAN form, via a no-catalog DummyHumanizer so ids are u<n> and columns #n) as its golden, submitting nothing. It takes the dataflow either inline (the create-dataflow body) or by reference — explain ref=<name> renders a dataflow a prior create-dataflow name=<name> declared, without repeating its body. join.spec declares the join, then explain ref=join asserts the differential-join plan alongside the count.

The multi-object render separates objects with blank lines, so the .spec format gains the datadriven doubled----- block form, emitted automatically by REWRITE.

🤖 Generated with Claude Code

@antiguru
antiguru marked this pull request as draft June 18, 2026 19:20
@antiguru
antiguru marked this pull request as ready for review June 18, 2026 19:29
@antiguru
antiguru requested a review from DAlperin June 18, 2026 19:29
@antiguru
antiguru force-pushed the headless-driver-explain branch from 3c46acd to 7dc1ed8 Compare June 19, 2026 08:02
Per review (DAlperin): a script using `optimize` asserts only the result, so
optimizer or lowering drift could silently change the plan under test. `explain`
renders the lowered LIR plan (the `EXPLAIN PHYSICAL PLAN` form, via a no-catalog
`DummyHumanizer` so ids are `u<n>` and columns `#n`) as its golden, submitting
nothing.

It takes the dataflow either inline (the same body as `create-dataflow`) or by
reference: `explain ref=<name>` renders a dataflow a prior `create-dataflow
name=<name>` declared, without repeating its body. `join.spec` declares the join,
then `explain ref=join` asserts the differential-join plan alongside the count.

The multi-object render separates objects with blank lines, so the `.spec` format
gains the `datadriven` doubled-`----` block form, emitted automatically by
`REWRITE`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@antiguru
antiguru force-pushed the headless-driver-explain branch from 7dc1ed8 to 3ca052e Compare August 26, 2026 09:31
@antiguru
antiguru enabled auto-merge (squash) August 26, 2026 09:48
@antiguru
antiguru merged commit cb4e73f into MaterializeInc:main Aug 26, 2026
18 checks passed
@def-

def- commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review

1. MEDIUM -- explain inherits ExplainConfig::default(), whose redacted flag follows the build profile

src/clusterd-test-driver/src/dataflow.rs:467

ExplainConfig::default() sets redacted: !mz_ore::assert::soft_assertions_enabled(), so an explain golden is rendered with literals redacted to in exactly the builds the scripts run in, and un-redacted under cargo test / PROFILE=dev / MZ_SOFT_ASSERTIONS=1. Any explain golden that contains a literal will both hide the constant it means to assert and differ between two supported ways of producing it.

Details

The chain: src/repr/src/explain.rs:212 derives redacted from soft_assertions_enabled(), and SOFT_ASSERTIONS (src/ore/src/assert.rs:66) defaults to cfg!(debug_assertions) unless MZ_SOFT_ASSERTIONS is set. The driver image is a plain cargo-build pre-image (src/clusterd-test-driver/ci/mzbuild.yml) at mzbuild's optimized/release profile, and HeadlessDriver (test/clusterd-test-driver/mzcompose.py:47) sets no MZ_SOFT_ASSERTIONS, so both the mzcompose run and run-local.py's default PROFILE=optimized render with redacted = true. The new explain_join_renders_plan unit test, running under cargo test, renders with redacted = false.

Nothing differs today: join.spec's plan has no literals, only ids, Arrange/Stream nodes and column keys. The first explain over a plan with a Filter, a Constant, a TopK limit or a non-column arrangement key gets Filter (#0 = █) instead of the value, silently unassertable — and a golden REWRITEn under PROFILE=dev will not match the mzcompose run. The doc comment's claim that the render is "stable" holds for ids and column references only.

Fix:

// Golden output must not depend on the build's soft-assertion setting, which
// `ExplainConfig::default()` derives `redacted` from.
let config = ExplainConfig {
    redacted: false,
    ..ExplainConfig::default()
};

@antiguru
antiguru deleted the headless-driver-explain branch August 26, 2026 10:45
@antiguru

Copy link
Copy Markdown
Member Author

Confirmed, and fixed in #38483 (this PR had already merged).

Reproduced the divergence: with MZ_SOFT_ASSERTIONS=0, a plan with a literal renders map=(█); with soft assertions on it renders the value. The fix pins redacted: false in an explain_config helper, so the render depends only on the plan and not on the build profile, plus a test that goes red without the pin.

Posted by Claude Code on behalf of @antiguru

antiguru added a commit to antiguru/materialize that referenced this pull request Aug 26, 2026
`ExplainConfig::default()` derives `redacted` from
`mz_ore::assert::soft_assertions_enabled()`, which follows `debug_assertions`
unless `MZ_SOFT_ASSERTIONS` overrides it. The `HeadlessDriver` mzcompose service
sets no such override and its image is built at mzbuild's release-like profile,
so `explain` rendered goldens with literals anonymized to a block character
there, while `cargo test` and `run-local.py`'s default `PROFILE=dev` rendered
them verbatim. A golden written under one would not match the other, and any
plan carrying a literal would assert a redaction instead of the constant.

Pinning `redacted: false` makes the render depend only on the plan. No test
covers it: every configuration the crate's unit tests run in already has soft
assertions on, so an assertion on the rendered text holds with or without the
pin, and the only way to make one bite is to flip the process-global
`SOFT_ASSERTIONS` atomic from inside the test.

Reported by def- on MaterializeInc#37141 and MaterializeInc#38483.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
antiguru added a commit that referenced this pull request Aug 26, 2026
Follow-up to #37141, addressing def-'s review that landed after the
merge.

`ExplainConfig::default()` derives `redacted` from
`mz_ore::assert::soft_assertions_enabled()`, which follows
`debug_assertions` unless `MZ_SOFT_ASSERTIONS` overrides it. The
`HeadlessDriver` mzcompose service sets no such override and its image
is a plain `cargo-build` pre-image at mzbuild's release-like profile, so
`explain` rendered goldens with literals anonymized to a block character
there, while `cargo test` and `run-local.py`'s default `PROFILE=dev`
rendered them verbatim. A golden written under one would not match the
other, and any plan carrying a literal would assert a redaction instead
of the constant. Nothing differs today because `join.spec`'s plan holds
only ids, `Arrange`/`Stream` nodes, and column keys.

Pinning `redacted: false` makes the render depend only on the plan and
not on how the binary was built. Confirmed by hand: with the pin removed
and `MZ_SOFT_ASSERTIONS=0`, a plan carrying a literal renders `map=(█)`.

No test covers this. Every configuration the crate's unit tests run in
already has soft assertions on, so an assertion on the rendered text
holds with or without the pin, and the only way to make one bite is to
flip the process-global `SOFT_ASSERTIONS` atomic from inside the test.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants