geoffreyclaude commented on code in PR #26242:
URL: https://github.com/apache/datafusion/pull/26242#discussion_r4292612413
##########
datafusion/physical-plan/src/sorts/sort.rs:
##########
@@ -793,22 +927,21 @@ impl ExternalSorter {
.map(|_| RecordBatchMemoryCounter::new())
}
- /// Sorts each run into its own stream, splitting `self.reservation`
between
- /// them.
+ /// Calculates each run's reservation size and reserves any shared input.
///
/// Runs can share buffers, for example zero-copy slices of one larger
batch,
/// and `self.reservation` covers each shared buffer once. A shared buffer
/// stays alive until the last run holding it is sorted, so if any buffers
are
/// shared, the reservation for every run's input is held until all the
runs
/// are sorted. Otherwise each run releases its input once it is sorted.
- fn sort_run_streams(
+ fn sort_run_reservations(
Review Comment:
Yes, other operators can hold the same buffers. I added a note in
5d19187433379dd6a5fbfcb0f328bda0fbbd818f that deduplication is local to these
runs, so reservations in different operators may overlap.
--
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]
