rluvaton commented on code in PR #23286:
URL: https://github.com/apache/datafusion/pull/23286#discussion_r3570408870
##########
datafusion/physical-plan/src/sorts/multi_level_merge.rs:
##########
@@ -311,6 +351,16 @@ impl MultiLevelMergeBuilder {
}
};
+ // Cap the merge output at the smallest limit among the runs
we're
+ // about to merge. Runs that were shrunk for skew carry a
smaller limit,
+ // if none do, every run carries `self.batch_size` and the
merge runs at
+ // the full batch size. The output stream is tagged with the
same limit
+ // (see the `MergeStep::Stream` returns below) so a re-spilled
+ // intermediate run stays shrunk and won't rebuild an
oversized batch on
+ // a later pass.
+ let output_batch_size =
+
min_batch_size_limit(&sorted_spill_files).unwrap_or(self.batch_size);
Review Comment:
We are already iterating over the spill files in `for (spill, _) in
sorted_spill_files {`
can you instead inline that?
--
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]
