fix: bound sort-merge join spill readback memory - #94
Merged
Merged
Conversation
Retain spill backing files and evict decoded payloads outside the next output working set. Preserve Full-join null output and deferred filter failures. Add exact spill/no-spill, memory-bound, revisit and cancellation coverage. Include the independent stale aggregate.slt REPLACE fixture repair needed by the full workspace gate; SQL planner behavior and the original SMJ RSS allowance remain unchanged. Standalone extended workspace, all-feature Clippy, full rust_lint.sh, and three original RSS guard repeats pass. The separate autoresearch runtime optimizations are excluded.
osipovartem
approved these changes
Sep 15, 2026
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.
Which issue does this PR close?
No existing issue. Reproduced by the fork's existing
smj_with_mem_limit_1RSS validation and the new focused regression below.Rationale for this change
A sort-merge join with a large equal-key group can spill during ingestion and
then progressively load the whole group back into memory. Already-visited
payloads remain decoded until the group is removed. On the pinned baseline,
the existing RSS guard fails in all three predetermined runs: 198.6, 197.7 and
198.2 MiB against its unchanged 152.6 MiB allowance.
With this standalone fix the same guard passes all three repeats at 84.5, 84.2
and 84.2 MiB. These are the guard's RSS increases over its reference query,
not absolute process RSS. Neither its assertions nor allocator settings were
changed to obtain this result.
What changes are included in this PR?
outside the next output's working set, releasing their reservations without
rewriting spill files.
of the dequeued head for deferred filter failures.
aggregate.slt: thepinned planner already rejects nonexistent
SELECT * REPLACEtargets. Updateits stale expected success to the exact existing error and retain a positive
grouped/HAVING replacement case. No planner/runtime SQL change or relaxed check.
The four separate autoresearch runtime optimizations are deliberately excluded.
Are these changes tested?
Local verification uses DataFusion
15ad0e236, Rust 1.97.0 and the application'sArrow/Parquet fork pin
cabd25a16, supplied through a Cargo patch configuration.The target fork branch is currently
31a96ad54(55.1); the touched runtime/testfiles are unchanged between these bases. This PR contains no Cargo pin change
or dependency downgrade. The target branch's eventual merge result is not
claimed as a locally tested tree; the results below are for the exact PR head.
against a 376,008-byte bound. All 72 SMJ tests pass with the fix.
full joins, revisits a 64-batch key group, and checks reservation cleanup after
completion and early cancellation.
cargo fmt --alland all-target/all-feature Clippy with-D warnings: pass.eight pre-existing ignored cases; all 508 SQL-logic files pass. Includes the
core integration RSS runners and 114 fuzz tests.
one SHA-verified executable.
The existing SMJ SQL benchmark also completed 24 runs (Q01/Q03/Q08/Q23, three
alternating baseline/fixed pairs), selecting SMJ and matching every expected row
count. With two partitions, batch size 8192 and a 1 GiB pool, median time changes were
−4.24%/−7.88%/+3.49%/+1.30%. These are an unoptimized CI-profile local screen,
not a release speedup claim; the small Full-join regressions are retained. This
screen does not quantify extra read I/O when revisiting large spilled groups.
The full unchanged
dev/rust_lint.shalso passes, including its alternate-featureClippy, TOML format, CI-pinned license/spelling checkers, Prettier, workflow check
and private-item workspace rustdoc with
-D warnings.Are there any user-facing changes?
Lower accumulation of decoded spill payloads, without a SQL/wire API change,
new runtime dependency or dataset-specific branch. Repeated streamed keys may
require extra spill reads after eviction: this is a memory/I/O tradeoff, not a
claim that every spilled join becomes faster. Join keys, match state, output
batches and the current restore window still consume memory; this is not a
strict total-process RSS or memory-pool cap.
This fix is separate from, and not included in, the frozen E21 local/SPCS
performance measurements.