dwsmith1983 commented on code in PR #25933:
URL: https://github.com/apache/datafusion/pull/25933#discussion_r4175826400
##########
datafusion/physical-plan/src/joins/sort_merge_join/bitwise_stream.rs:
##########
@@ -669,7 +669,11 @@ impl BitwiseSortMergeJoinStream {
let inner_batch = self.inner_batch.as_ref().unwrap();
let slice = inner_batch.slice(from, group_end - from);
- self.inner_buffer_size += slice.get_array_memory_size();
+ // A slice reports its parent batch's full buffers, so charge only
+ // the rows the group holds. View arrays still count their parent's
+ // data buffers, and a group spanning an inner batch boundary keeps
+ // the earlier parent batch alive, so this can be one batch low.
+ self.inner_buffer_size += slice.get_sliced_size()?;
Review Comment:
Thanks, good catch. Applied in 460cb8ca8 rather than leaving it for later: a
group reaching the batch end is charged its full parent again, and the
sliced-size test now allows one spill per inner batch (2/2/1/1, as you
measured). I also added
`bitwise_key_group_at_inner_batch_end_charged_full_parent`, which checks
`peak_mem_used` covers a whole inner batch when every key is unique; it reports
12 bytes against 12,576 without the change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
