Skip to content

test: add Poll::Pending spill stream coverage for async spill re-entry paths#23353

Open
pantShrey wants to merge 4 commits into
apache:mainfrom
pantShrey:test/pending_stream_smj_test
Open

test: add Poll::Pending spill stream coverage for async spill re-entry paths#23353
pantShrey wants to merge 4 commits into
apache:mainfrom
pantShrey:test/pending_stream_smj_test

Conversation

@pantShrey

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

The SortMergeJoin refactor in #22230 introduced async spill read paths in both materializing_stream.rs (poll_spilled_batches) and bitwise_stream.rs (the spilled-batch loop in process_key_match_with_filter). Both call SpillFile::read_stream() and poll the resulting byte stream, which can return Poll::Pending.

The existing spill tests exercise spilling and restoring batches, but always run against the default local-file SpillFile, whose read_stream() happens to resolve synchronously on first poll in practice. As a result, the re-entry logic in both streams has no dedicated test coverage.

What changes are included in this PR?

  • Adds a PendingSpillFile / PendingTempFileFactory pair in sort_merge_join/tests.rs that wraps the default local SpillFile backend (via DiskManagerMode::Custom). Every spill read splits the real spill file's bytes into small fixed-size chunks and yields Poll::Pending before each chunk, forcing SpillReaderStream's IPC decoder to genuinely suspend mid-read and resume from partially buffered state across multiple real Pending cycles, rather than completing on the first poll.
  • Adds materializing_spill_pending_stream, covering Inner/Left/ Right/Full joins (the BufferedBatchState::Spilled restore path in poll_spilled_batches).
  • Adds bitwise_spill_pending_stream, covering LeftSemi/LeftAnti/ RightSemi/RightAnti joins with a filter (the inner_key_spill re-entry path in process_key_match_with_filter).
  • Both tests compare the Pending-forced spilled result against a no-spill run of the same join, asserting the results are identical, so any state corruption introduced by suspending mid-read would be caught.

Are these changes tested?

Yes -- this is a PR to test recent changes

Are there any user-facing changes?

No.

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

Labels

physical-plan Changes to the physical-plan crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Poll::Pending spill stream test coverage for SMJ async spill paths

1 participant