kumarUjjawal commented on code in PR #25914:
URL: https://github.com/apache/datafusion/pull/25914#discussion_r4162994036


##########
datafusion/physical-plan/src/aggregates/group_values/row.rs:
##########
@@ -283,10 +283,15 @@ impl GroupValues for GroupValuesRows {
     }
 
     fn clear_shrink(&mut self, num_rows: usize) {
-        self.group_values = self.group_values.take().map(|mut rows| {
-            rows.clear();
-            rows
-        });
+        self.group_values = if num_rows == 0 {

Review Comment:
   clear_shrink(0) does not release all extra memory. This clears group_values, 
but keeps the memory in rows_buffer. Batches with more than 1,000 rows or large 
values make this buffer grow. After clear_shrink(0), that extra memory remains 
allocated. This leaves less memory available to sort data during spill.
   Recreate rows_buffer when num_rows == 0. Extend the test with a batch that 
exceeds the initial buffer capacity.



-- 
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]

Reply via email to