viirya commented on code in PR #25428:
URL: https://github.com/apache/datafusion/pull/25428#discussion_r4128188670
##########
datafusion/physical-plan/src/sorts/multi_level_merge.rs:
##########
@@ -393,6 +411,35 @@ impl MultiLevelMergeBuilder {
}
};
+ if self.size_intermediate_merges
+ && self.sorted_streams.is_empty()
+ && !self.sorted_spill_files.is_empty()
+ {
+ // If one intermediate merge can leave a final pass with
the
+ // admitted fan-in, merge only the runs needed to get
there.
+ // Keep the admitted reservation so sizing does not change
Review Comment:
Thanks — the replay-headroom point is exactly what my suggestion missed.
Budgeting the final pass within the retained grant is the right way to close it.
##########
datafusion/physical-plan/src/sorts/multi_level_merge.rs:
##########
@@ -985,6 +1033,148 @@ mod tests {
)
}
+ #[rstest::rstest]
+ #[case::full_batches(9, 128, 3)]
+ #[case::unchanged_selection(13, 128, 7)]
+ #[case::short_batches(9, 64, 7)]
+ #[tokio::test]
+ async fn intermediate_merge_sizing_preserves_final_output(
Review Comment:
Covering both release points is what I was after, and having the nine-run
cases fail on the previous head makes this a real regression test. Thanks for
adding it.
--
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]
