crepererum commented on code in PR #4924:
URL: https://github.com/apache/arrow-datafusion/pull/4924#discussion_r1072300782
##########
datafusion/core/src/physical_plan/aggregates/row_hash.rs:
##########
@@ -70,29 +74,33 @@ use hashbrown::raw::RawTable;
///
/// [Compact]: datafusion_row::layout::RowType::Compact
/// [WordAligned]: datafusion_row::layout::RowType::WordAligned
-pub(crate) struct GroupedHashAggregateStreamV2 {
+pub(crate) struct GroupedHashAggregateStream {
stream: BoxStream<'static, ArrowResult<RecordBatch>>,
schema: SchemaRef,
}
-/// Actual implementation of [`GroupedHashAggregateStreamV2`].
+/// Actual implementation of [`GroupedHashAggregateStream`].
///
/// This is wrapped into yet another struct because we need to interact with
the async memory management subsystem
/// during poll. To have as little code "weirdness" as possible, we chose to
just use [`BoxStream`] together with
-/// [`futures::stream::unfold`]. The latter requires a state object, which is
[`GroupedHashAggregateStreamV2Inner`].
-struct GroupedHashAggregateStreamV2Inner {
+/// [`futures::stream::unfold`]. The latter requires a state object, which is
[`GroupedHashAggregateStreamInner`].
+struct GroupedHashAggregateStreamInner {
schema: SchemaRef,
input: SendableRecordBatchStream,
mode: AggregateMode,
- aggr_state: AggregationState,
- aggregate_expressions: Vec<Vec<Arc<dyn PhysicalExpr>>>,
+ normal_aggr_expr: Vec<Arc<dyn AggregateExpr>>,
+ row_aggr_state: RowAggregationState,
+ /// Aggregate expressions not supporting row accumulation
+ normal_aggregate_expressions: Vec<Vec<Arc<dyn PhysicalExpr>>>,
Review Comment:
I.e. I'm personally convinced that the row accumulator style is a dead end.
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
