alamb commented on code in PR #6904:
URL: https://github.com/apache/arrow-datafusion/pull/6904#discussion_r1258697406
##########
datafusion/physical-expr/src/aggregate/groups_accumulator/accumulate.rs:
##########
@@ -0,0 +1,879 @@
+// Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
This file contains the templated, inner loops for the accumulators that
handles filtering and nulls with specialized loops. I think it could be made
faster with some more specialization but I think the complexity might have
diminishing returns.
##########
datafusion/execution/src/memory_pool/proxy.rs:
##########
@@ -44,6 +49,9 @@ impl<T> VecAllocExt for Vec<T> {
self.push(x);
}
+ fn allocated_size(&self) -> usize {
+ std::mem::size_of::<T>() * self.capacity()
Review Comment:
I refactored this calculation into its own function as leaving it inline in
the group code made it harder to follow in my opinion
##########
datafusion/physical-expr/src/aggregate/min_max.rs:
##########
@@ -1022,6 +1112,224 @@ impl RowAccumulator for MinRowAccumulator {
}
}
+trait MinMax {
Review Comment:
@tustvold do you know of any pre-existing trait that will provide a min
based on type?
--
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]
