Repository navigation
fix(query): expression bindings unify; reorder switch and plan-independence test (#596) - #601
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HyLscCUuJ1GQpdAeX79EdR
optimizer::reorder() replaces every cfg(not(feature = "wasm")) reordering site: pattern selectivity sort, not/not-join cost sorts, or/or-join branch sorts and their #250 short-circuits, and the engine's HashJoin build side (off: build on the inner pattern). Release builds compile a constant. Under cfg(test) or test-oracle a thread-local switch turns it off; minigraf::__reorder exposes it to integration tests (test-oracle only). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HyLscCUuJ1GQpdAeX79EdR
[(f …) ?x] used to overwrite ?x wherever the planner ran it, so when a pattern also bound ?x the answer depended on the plan (native vs the wasm feature). It now keeps only the rows where its result equals ?x's value, with the join equality of #545 (IEEE == floats, keyword equals the ref of the entity it names, int never equals float, NaN equals nothing), in the executor's plan loops, or branches, not bodies, the evaluator and the engine's Step::Bind (Out::New/Check/Replace). Binders records expressions by text position: the first binder in :where gives the value. tests/plan_independence_test.rs (test-oracle) runs a query matrix with reordering on and off through execute and a cursor; the model-based and property tests rerun every query with reordering off. Gates the #250 short-circuit tests to non-wasm (they already failed under --features wasm). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HyLscCUuJ1GQpdAeX79EdR
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
PR 1 of 3 for #596 / #594 (plan:
docs/superpowers/plans/2026-10-15-594-596-plan-independence.md, spec:docs/superpowers/specs/2026-10-15-plan-independent-answers-design.md). #596 is closed by hand after merge (v3does not fire closing keywords).What changes
[(f …) ?x]whose?xanother clause binds keeps only the rows where the result equals?x's value, with the streaming engine 3/9: IndexNestedLoopJoin and HashJoin (#473) #545 join equality (IEEE==floats, keyword = ref of the entity it names, int ≠ float, NaN ≠ anything). Applies to the executor's two plan loops,orbranches andnotbodies (apply_expr_clauses), the evaluator (recursive and stratified paths,evaluate_not_join) and the engine'sStep::Bind(Out::New/Check/Replace). One helper,executor::apply_expr, replaces five copies of the overwrite logic.Bindersnow records expressions by:whereposition, so an expression earlier in the text than the pattern that bound the variable gives its value (matters only for keyword vs ref and-0.0vs0.0).optimizer::reorder()replaces every#[cfg(not(feature = "wasm"))]reordering site: pattern selectivity sort,not/not-joincost sorts (planner, deferred, stratified evaluator),or/or-joinbranch sorts and the perf: short-circuit or/or-join branch evaluation using cost model #250 short-circuits, and the engine'sHashJoinbuild side (switch off: build on the inner pattern;wasmkeeps the cost choice). Release builds compile a constant.minigraf::__reorder(bool) -> ReorderGuardis#[doc(hidden)]and exists only undertest-oracle.Tests
tests/plan_independence_test.rs(7 tests,test-oracleonly): a query matrix (joins in both text orders, cross products, keyword/ref, floats, long strings, expressions, recursive rules and rules withnot,or/or-joinincluding perf: short-circuit or/or-join branch evaluation using cost model #250 shapes, placed and deferrednot/not-join, aggregates,:as-of,:valid-at,:any-valid-time), in memory and on file after checkpoint and reopen, throughexecuteand a cursor, with reordering on and off: all four bags must match. Plus Expression binding a variable a pattern also binds depends on plan order #596's example and the unification cases. Before the fix, Expression binding a variable a pattern also binds depends on plan order #596's example failed (3 rows vs 2).test-oracle(through aCtx::rerunhook, so theops_sequencefuzz target, which sharesreference_model, is unaffected;cargo checkinfuzz/passes).apply_expr(equal/different, int vs float, NaN,-0.0/0.0with and without replace, keyword/ref both ways, failing expression),Binders::expression, the switch (source order, per thread, restore), engineBindmodes and build side; six unification shapes added to the engine's join matrix.join_rules_test::expression_binding_is_kept_by_later_joinsencoded the old overwrite rule; renamed and now checks the first-binder rule both ways.wasm: threedisjunction_testtests of the perf: short-circuit or/or-join branch evaluation using cost model #250 short-circuits, which are native-only. They already failed undercargo test --features wasmonv3(verified on a cleanorigin/v3checkout); CI does not run that configuration.Local:
cargo test1514 (1504 passed, 10 ignored);cargo test --features test-oracle1510 passed, 11 ignored;cargo test --features wasmpasses; clippy clean with--all-features, default,wasm,test-oracle, wasm32browser, wasm32-wasip1. Codex review: no findings.Docs
CHANGELOG (Breaking changes + Tests; the #545 entry's expression sentence now points to the new entry), CLAUDE.md, README, TEST_COVERAGE. Docs site: project-minigraf/minigraf-docs PR (expression clauses,
@since v3.0.0).Plan notes
The plan adds PR 3: drop the
wasmgate so every build reorders and keep thewasmfeature as a deprecated no-op (no first-party build enables it;minigraf-wasmusesbrowser, andwasm.ymlbuilds wasm32 without it).🤖 Generated with Claude Code
https://claude.ai/code/session_01HyLscCUuJ1GQpdAeX79EdR