andygrove commented on code in PR #26154:
URL: https://github.com/apache/datafusion/pull/26154#discussion_r4262159209
##########
datafusion/functions-aggregate/src/min_max/min_max_bytes.rs:
##########
@@ -416,9 +416,10 @@ struct MinMaxBytesState {
min_max: Vec<Option<Vec<u8>>>,
/// The data type of the array
data_type: DataType,
- /// The total bytes of the string data (for pre-allocating the final array,
- /// and tracking memory usage)
+ /// The total length of the string data, for pre-allocating the final
array.
total_data_bytes: usize,
+ /// The retained inner vector capacities, cached to keep `size()` O(1).
+ total_data_capacity: usize,
Review Comment:
A small readability point, not blocking: this field is the retained
allocation, while `emit_to` still calls the summed lengths
`first_data_capacity` and documents its return value as `data_capacity`, so
"capacity" now means two different things in the same function. Renaming this
to something like `total_allocated_bytes`, or the local to `first_data_bytes`,
would make that clearer.
--
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]
