Jefffrey commented on code in PR #19278:
URL: https://github.com/apache/datafusion/pull/19278#discussion_r2610622476
##########
datafusion/functions-aggregate/src/median.rs:
##########
@@ -289,14 +289,32 @@ impl<T: ArrowNumericType> Accumulator for
MedianAccumulator<T> {
}
fn evaluate(&mut self) -> Result<ScalarValue> {
- let d = std::mem::take(&mut self.all_values);
+ let d = self.all_values.clone();
Review Comment:
My gut feeling says this would be a noticeable performance regression, as
median already can be memory intensive since it holds all values before
performing the evaluate.
Perhaps we can modify the `calculate_median` method to accept a `&mut`
instead of taking by value?
--
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]
