zhuqi-lucas commented on code in PR #15380:
URL: https://github.com/apache/datafusion/pull/15380#discussion_r2040673419


##########
datafusion/physical-plan/src/sorts/sort.rs:
##########
@@ -645,7 +645,36 @@ impl ExternalSorter {
             return self.sort_batch_stream(batch, metrics, reservation);
         }
 
-        let streams = std::mem::take(&mut self.in_mem_batches)
+        let mut merged_batches = Vec::new();
+        let mut current_batches = Vec::new();
+        let mut current_size = 0;
+
+        for batch in std::mem::take(&mut self.in_mem_batches) {

Review Comment:
   > I think it would be nice to use `pop` (`while let Some(batch) = v.pop`) 
here to remove the batch from the vec once sorted to reduce memory usage. Now 
the batch is AFAIK retained until after the loop.
   
   Thank you @Dandandan for review and good suggestion, addressed your 
suggestion!



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to