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


##########
datafusion/physical-plan/src/aggregates/aggregate_hash_table/common_ordered.rs:
##########
@@ -304,18 +313,18 @@ impl<AggrMode> OrderedAggregateTable<AggrMode> {
             EmitTo::All => (EmitTo::First(self.batch_size), false),
         }
     }
-    /// Aggregates one evaluated input batch.
-    ///
-    /// This common utility is used by ordered partial and ordered final 
aggregation.
-    ///
-    /// # Argument: `is_final`
+
+    /// Aggregates one evaluated input batch after selecting the mode-specific
+    /// accumulator operation.
     ///
-    /// - `true`: merge partial aggregate states for final aggregation.
-    /// - `false`: update aggregate states from raw input for partial 
aggregation.
+    /// Each aggregation mode chooses a different `aggregate_fn` according to 
its
+    /// semantics. For example, partial aggregation takes raw inputs and 
updates
+    /// stored partial states, so it uses
+    /// [`datafusion_expr::GroupsAccumulator::update_batch`].
     pub(super) fn aggregate_evaluated_batch(
         &mut self,
         evaluated_batch: &EvaluatedAggregateBatch,
-        is_final: bool,
+        aggregate_fn: AggregateBatchFn,

Review Comment:
   here is the key change for this file, just a clean-up to make it easier to 
be reused by different aggregation modes.



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