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


##########
datafusion/core/src/physical_plan/aggregates/bounded_aggregate_stream.rs:
##########
@@ -325,18 +328,22 @@ impl RecordBatchStream for BoundedAggregateStream {
 /// indices for a group. This information is used when executing streaming
 /// GROUP BY calculations.
 struct GroupOrderInfo {
+    /// The group by key
     owned_row: OwnedRow,
+    /// the hash value of the group
     hash: u64,
+    /// the range of row indices in the input batch that belong to this group
     range: Range<usize>,
 }
 
 impl BoundedAggregateStream {
-    // Update the aggr_state according to group_by values (result of 
group_by_expressions) when group by
-    // expressions are fully ordered.
-    fn update_ordered_group_state(
+    /// Update the aggr_state hash table according to group_by values
+    /// (result of group_by_expressions) when group by expressions are
+    /// fully ordered.
+    fn update_fully_ordered_group_state(
         &mut self,
         group_values: &[ArrayRef],
-        per_group_indices: Vec<GroupOrderInfo>,
+        per_group_order_info: Vec<GroupOrderInfo>,

Review Comment:
   I found the old name 'group_indices` confusing here as it is not just indices



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

Reply via email to