You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/06/15 20:36:55 UTC

[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #6689: removed self.all_values.len() from inside reserve

alamb commented on code in PR #6689:
URL: https://github.com/apache/arrow-datafusion/pull/6689#discussion_r1231511337


##########
datafusion/physical-expr/src/aggregate/median.rs:
##########
@@ -126,7 +126,7 @@ impl Accumulator for MedianAccumulator {
         let array = &values[0];
 
         assert_eq!(array.data_type(), &self.data_type);
-        self.all_values.reserve(self.all_values.len() + array.len());
+        self.all_values.reserve(array.len());

Review Comment:
   Per the rust docs: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.reserve
   
   > Reserves capacity for at least additional more elements to be inserted in the given Vec<T>. The collection may reserve more space to speculatively avoid frequent reallocations. After calling reserve, capacity will be greater than or equal to self.len() + additional. Does nothing if capacity is already sufficient.
   
   👍 



-- 
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-unsubscribe@arrow.apache.org

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