chore: fold #663 into 0.18.9, revert premature 0.18.10 bump - #671
Merged
Merged
Conversation
This was referenced Sep 4, 2026
iheitlager
added a commit
that referenced
this pull request
Sep 7, 2026
…#691) * feat: streaming Execution primitive, with run() as its wrapper (#683) Every execution entry point materialized a whole `Vec<Vec<Value>>` before returning, so reading row 0 of a large result cost building row N and a caller could not stop early. Spike 014 (#682) measured that at 137.7 MB peak heap and 5.36 ms to first row for a 1,000,000-row result, against 8.68 MB and 44.7 us streaming. It was also not fixable from outside the crate: `fn dispatch` and `fn run` are private, and `ResultRow` hands its row to `Vm::emit_row`, which pushes into a `Vec` inside the `Vm`. Adds `Execution` (`new`/`next_row`/`autocommit`) and reimplements `run()` as a wrapper that collects `next_row` into the same `Vec` it already returned. The wrapper is the load-bearing part: batch and streaming become literally the same loop, so they cannot drift, and the existing suite is the equivalence proof — 1562 passed / 0 failed before, 1568 / 0 after, the delta being exactly this ticket's six new tests. `pending` is a FIFO rather than a pop off the back because `pragma::integrity_check` emits one row per problem from a single dispatch, and draining from the back would silently reverse that output. No test in the suite reached that path before now, so `vdbe_streaming_execution_test.rs` empties three indexes to force a genuinely multi-row result — and the guard was mutation-checked: with the FIFO replaced by a pop, that test and only that test fails. Deliberately not included: any transaction-aware streaming constructor (`Vm::autocommit` is private, and read-only streaming is what Req 7 targets), and chunking, which #682 showed is a transport concern for the facade rather than the primitive. ADR-0038 records both, plus why a second parallel execution path was rejected. No CHANGELOG entry or version bump: this repo folds those into a separate `chore/*-fold-into-0.18.x` PR (e.g. #671), and ticket PRs do not carry them. #683's acceptance criteria said otherwise and were wrong about the convention. Refs: 013/Req-7, #683, #682, #678 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(fuzz): bound vdbe_exec's row drain so an emit-loop can't OOM the gate `vdbe_exec` feeds arbitrary bytecode to `execute()`, which hands back every row the program emitted. The decoder is free to build `ResultRow` with a register range up to `MAX_REGISTERS` and an `Init` whose negative P2 lands back on it (`to_pc` clamps to 0), so the result set grows without bound — 4.3 GB in ~1400 rows of 131,072 NULLs each, tripping libFuzzer's 4096 MB limit (CI run 33857317005). Not a regression from this branch. Replaying the artifact against `origin/main` grows at the same ~1 GB/s, because nothing drains `Vm::rows` there either; the streaming rewrite moves rows through a FIFO but `run()` still collects them all. The fuzzer only reached it now because its writable corpus is gitignored, so every CI run re-explores from the three committed seeds on a fresh random seed. Bounding accumulation in the engine would be the wrong fix — a `SELECT` that legitimately returns N rows must be allowed to return N rows, and real programs come from codegen, never from a caller handing the VM raw bytecode. So the bound goes in the harness: rows are pulled through `Execution` and dropped as they arrive, and `MAX_ROWS` caps the emitted work per input so a wide `ResultRow` cannot exhaust the budget in time instead of memory. Per ADR-0040 `run()` is this same loop plus a `Vec::push`, so the dispatch coverage that spec 009's no-panic-totality obligation (#89) is actually about is unchanged. The OOM input is committed as seed `result_row_emit_loop` per tests/fuzz/seeds/README.md, so the regression cannot come back unnoticed. Verified: the seed goes from SIGKILL to 175 ms; `make fuzz-smoke` is green across all seven targets; a 90s `vdbe_exec` run peaks at 360 MB. Throughput is unaffected by the cap (MAX_ROWS 8 vs 64 measured within noise), so 64 is kept for multi-row coverage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Ilja Heitlager <iheitlager@schubergphilis.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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Cargo.toml/README.mdback to 0.18.9.Test plan
cargo build— Cargo.lock regenerated at 0.18.9cargo clippy/cargo fmt --check(pre-push hook) — cleanspend: trivial