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


##########
datafusion/physical-plan/src/aggregates/mod.rs:
##########
@@ -846,30 +847,55 @@ impl LimitOptions {
     }
 }
 
-/// Aggregation state, separating a DISTINCT soft limit from accumulators and 
filters.
+/// Mutually exclusive aggregation implementations and their configuration.
+///
+/// # Public Only for Internal Use:
+/// `datafusion-physical-optimizer` inspects and combines aggregate kinds.
+/// This enum is not part of the supported public API.
+#[doc(hidden)]
 #[derive(Debug, Clone)]
-enum AggregateKind {
-    /// Ordinary aggregation, including the existing Top-K configuration.
+pub enum AggregateKind {
+    /// Ordinary aggregation, with no limit on the groups retained.
     General {
         group_by: Arc<PhysicalGroupBy>,
         aggr_expr: Arc<[Arc<AggregateFunctionExpr>]>,
         filter_expr: Arc<[Option<Arc<dyn PhysicalExpr>>]>,
-        limit_options: Option<LimitOptions>,
     },
     /// `SELECT DISTINCT k FROM t LIMIT n`: eligible streams may stop after n 
groups.
     /// Other streams consume all input; the parent LIMIT enforces the row 
count.
     DistinctLimit {
         group_by: Arc<PhysicalGroupBy>,
         limit: usize,
     },
+    /// See [`AggregateExec::try_optimize_topk`] for details.
+    TopKMinMax {
+        group_by: Arc<PhysicalGroupBy>,
+        aggr_expr: Arc<AggregateFunctionExpr>,
+        limit: usize,
+        descending: bool,
+        nulls_first: bool,
+    },
+    /// See [`AggregateExec::try_optimize_topk`] for details.
+    TopKDistinct {
+        group_by: Arc<PhysicalGroupBy>,
+        limit: usize,
+        descending: bool,
+    },
 }
 
 /// Hash aggregate execution plan
 #[derive(Debug, Clone)]
 pub struct AggregateExec {
     /// Aggregation mode (full, partial)
     mode: AggregateMode,
-    kind: AggregateKind,
+    /// Aggregation implementation and its configuration.
+    ///
+    /// # Public Only for Internal Use:
+    /// `datafusion-physical-optimizer` updates this when combining aggregates.
+    /// Changes must preserve the expressions, schema, and plan properties.

Review Comment:
   Yes we can remove this `pub` after this cleanup: 
https://github.com/apache/datafusion/pull/25696/changes#r4100893886



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