2010YOUY01 commented on code in PR #25312:
URL: https://github.com/apache/datafusion/pull/25312#discussion_r4055971563


##########
datafusion/physical-plan/src/aggregates/aggregate_hash_table/ordered_partial_table.rs:
##########
@@ -89,26 +89,22 @@ impl OrderedAggregateTable<PartialMarker> {
         )
     }
 
-    /// Emits the next batch of partial state rows for groups proven complete 
by
-    /// the input ordering.
-    ///
-    /// For example, when the query is `GROUP BY a` and the input is ordered by
-    /// `a`, seeing a latest input row with `a = 3` means all groups with `a < 
3`
-    /// are complete and safe to emit.
-    ///
-    /// Key steps:
-    /// 1. Ask `group_ordering` to decide how many groups can be emitted 
eagerly.
-    /// 2. Remove the emitted groups from `group_ordering`, `GroupValues`, and
-    ///    all `GroupsAccumulator`s.
-    ///
-    /// This may output small batches. Avoiding tiny batches is left to future
-    /// ordered-aggregation optimizations.
-    pub(in crate::aggregates) fn next_output_batch(
+    /// Materializes all groups proven complete by the input ordering, leaving
+    /// the active ordered-key range in the table.
+    pub(in crate::aggregates) fn take_completed_state_batch(
         &mut self,
     ) -> Result<Option<RecordBatch>> {
-        self.next_output_batch_inner(
+        if self.is_empty() {
+            return Ok(None);
+        }
+        let Some(emit_to) = self.group_ordering().emit_to() else {
+            return Ok(None);
+        };
+        self.materialize_groups(
+            emit_to,
             HashAggregateAccumulator::state,
             AccumulatorPhase::State,
         )

Review Comment:
   I'd like to skip this one due to it's not easy to parse -- it goes into a 
lot of detail instead of being directly to the key point. Working through it 
might not be the best use of my time.
   
   - related discussion: https://github.com/apache/datafusion/issues/25317
   
   but i'm happy to follow-up if there is an e2e reproducer for such violation.



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

Reply via email to