Skip to content

Commit 4ca85d2

Browse files
authored
refactor(hash-aggr): Simplify aggregate hash table with tempated functions (#23324)
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123. --> - Part of #22710 - An alternative for and closes #23309 ## Rationale for this change <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> See #23309 and #23309 (comment) for background. I prefer this approach, but I want to point out the tradeoff for this PR's approach: the shared utility includes a complex lambda function argument, this makes them harder to extend. But I think it's okay since most functionality has been implemented for the refactor, and there are not likely to have new functional requirements. ## What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> ## Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> ## Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> <!-- If there are any breaking changes to public APIs, please add the `api change` label. -->
1 parent 8139397 commit 4ca85d2

4 files changed

Lines changed: 139 additions & 252 deletions

File tree

datafusion/physical-plan/src/aggregates/aggregate_hash_table/common.rs

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,95 @@ impl<AggrMode> AggregateHashTable<AggrMode> {
171171
})
172172
}
173173

174+
/// Aggregates one input batch after selecting the mode-specific accumulator
175+
/// operation.
176+
///
177+
/// Each aggregation mode chooses a different `aggregate_fn` according to its
178+
/// semantics. For example, partial aggregation takes raw inputs, and update them
179+
/// into stored partial states, so [`GroupsAccumulator::update_batch`] is used.
180+
pub(super) fn aggregate_batch_inner(
181+
&mut self,
182+
batch: &RecordBatch,
183+
aggregate_fn: AggregateBatchFn,
184+
) -> Result<()> {
185+
let evaluated_batch = self.evaluate_batch(batch)?;
186+
let state = self.state.building_mut();
187+
188+
let _timer = self.group_by_metrics.aggregation_time.timer();
189+
for group_values in &evaluated_batch.grouping_set_args {
190+
state
191+
.group_values
192+
.intern(group_values, &mut state.batch_group_indices)?;
193+
let group_indices = &state.batch_group_indices;
194+
let total_num_groups = state.group_values.len();
195+
196+
for (acc, values) in state
197+
.accumulators
198+
.iter_mut()
199+
.zip(evaluated_batch.accumulator_args.iter())
200+
{
201+
aggregate_fn(acc, values, group_indices, total_num_groups)?;
202+
}
203+
}
204+
205+
Ok(())
206+
}
207+
208+
/// Materializes the full output once, then returns it downstream incrementally
209+
/// by slicing it into `batch_size` chunks.
210+
///
211+
/// Each aggregation mode chooses a different `materialize_accumulator_fn`
212+
/// according to its semantics. For example, partial aggregation emits
213+
/// partial states to feed the final stage, so it uses [`GroupsAccumulator::state`].
214+
///
215+
/// This is a temporary solution until blocked state management is implemented:
216+
/// Issue: <https://github.com/apache/datafusion/issues/7065>
217+
pub(super) fn next_output_batch_inner(
218+
&mut self,
219+
materialize_accumulator_fn: MaterializeAccumulatorFn,
220+
) -> Result<Option<RecordBatch>> {
221+
let output_schema = Arc::clone(&self.output_schema);
222+
let batch_size = self.batch_size;
223+
224+
let mut output =
225+
match std::mem::replace(&mut self.state, AggregateHashTableState::Done) {
226+
AggregateHashTableState::Outputting(mut state) => {
227+
if state.group_values.is_empty() {
228+
return Ok(None);
229+
}
230+
231+
// Accumulator output consumes internal state. Materialize all
232+
// groups once, then slice the materialized batch on later polls.
233+
let emit_to = EmitTo::All;
234+
let timer = self.group_by_metrics.emitting_time.timer();
235+
let mut columns = state.group_values.emit(emit_to)?;
236+
for acc in state.accumulators.iter_mut() {
237+
columns.extend(materialize_accumulator_fn(acc, emit_to)?);
238+
}
239+
drop(timer);
240+
241+
let batch = RecordBatch::try_new(output_schema, columns)?;
242+
debug_assert!(batch.num_rows() > 0);
243+
MaterializedAggregateOutput::new(batch)
244+
}
245+
AggregateHashTableState::OutputtingMaterialized(output) => output,
246+
AggregateHashTableState::Done => return Ok(None),
247+
AggregateHashTableState::Building(_) => {
248+
return internal_err!(
249+
"next_output_batch must be called in the outputting state"
250+
);
251+
}
252+
};
253+
254+
let batch = output.next_batch(batch_size);
255+
if output.is_exhausted() {
256+
self.state = AggregateHashTableState::Done;
257+
} else {
258+
self.state = AggregateHashTableState::OutputtingMaterialized(output);
259+
}
260+
Ok(batch)
261+
}
262+
174263
pub(in crate::aggregates) fn memory_size(&self) -> usize {
175264
match &self.state {
176265
AggregateHashTableState::Building(state)
@@ -241,6 +330,31 @@ pub(super) struct HashAggregateAccumulator {
241330

242331
pub(super) type AggregateAccumulator = HashAggregateAccumulator;
243332

333+
/// Function used by [`AggregateHashTable::aggregate_batch_inner`] to update one
334+
/// accumulator with one evaluated input batch.
335+
///
336+
/// Arguments:
337+
/// * accumulator to update.
338+
/// * accumulator's evaluated arguments and optional filter.
339+
/// * one group index per input row, mapping each row to its interned group.
340+
/// * total number of groups currently interned in that buffer, including newly
341+
/// interned groups.
342+
pub(super) type AggregateBatchFn = fn(
343+
&mut AggregateAccumulator,
344+
&EvaluatedAccumulatorArgs,
345+
&[usize],
346+
usize,
347+
) -> Result<()>;
348+
349+
/// Function used by [`AggregateHashTable::next_output_batch_inner`] to
350+
/// materialize one accumulator's output columns.
351+
///
352+
/// Arguments:
353+
/// * accumulator to materialize.
354+
/// * group range to emit from the accumulator.
355+
pub(super) type MaterializeAccumulatorFn =
356+
fn(&mut AggregateAccumulator, EmitTo) -> Result<Vec<ArrayRef>>;
357+
244358
/// Evaluated aggregate arguments and filter for one input batch.
245359
///
246360
/// For example, `AVG(x + 1) FILTER (WHERE x > 0)` evaluates both `x + 1`
@@ -439,6 +553,13 @@ impl HashAggregateAccumulator {
439553
self.accumulator.evaluate(emit_to)
440554
}
441555

556+
pub(super) fn evaluate_to_columns(
557+
&mut self,
558+
emit_to: EmitTo,
559+
) -> Result<Vec<ArrayRef>> {
560+
Ok(vec![self.evaluate(emit_to)?])
561+
}
562+
442563
/// Evaluating partial aggregate results according to `EmitTo`, and reset inner
443564
/// states. (e.g. after `state(EmitTo::All)`, it returns all accumulated groups
444565
/// , and clear the inner buffers)

datafusion/physical-plan/src/aggregates/aggregate_hash_table/final_table.rs

Lines changed: 6 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,13 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18-
use std::sync::Arc;
19-
2018
use arrow::datatypes::SchemaRef;
2119
use arrow::record_batch::RecordBatch;
22-
use datafusion_common::{Result, internal_err};
23-
use datafusion_expr::EmitTo;
20+
use datafusion_common::Result;
2421

2522
use crate::aggregates::AggregateExec;
2623

27-
use super::common::{
28-
AggregateHashTable, AggregateHashTableBuffer, AggregateHashTableState, FinalMarker,
29-
MaterializedAggregateOutput,
30-
};
24+
use super::common::{AggregateHashTable, FinalMarker, HashAggregateAccumulator};
3125

3226
/// Implementation specific to final aggregation, where the table stores partial
3327
/// aggregate states and the input rows are also partial states.
@@ -61,90 +55,16 @@ impl AggregateHashTable<FinalMarker> {
6155
pub(in crate::aggregates) fn next_output_batch(
6256
&mut self,
6357
) -> Result<Option<RecordBatch>> {
64-
let output_schema = Arc::clone(&self.output_schema);
65-
let batch_size = self.batch_size;
66-
// Take ownership of the output state. `emit_next_materialized_batch`
67-
// restores `self.state` to `OutputtingMaterialized` or `Done`.
68-
match std::mem::replace(&mut self.state, AggregateHashTableState::Done) {
69-
AggregateHashTableState::Outputting(state) => {
70-
if state.group_values.is_empty() {
71-
return Ok(None);
72-
}
73-
74-
let output = self.materialize_final_output(state, output_schema)?;
75-
Ok(self.emit_next_materialized_batch(output, batch_size))
76-
}
77-
AggregateHashTableState::OutputtingMaterialized(output) => {
78-
Ok(self.emit_next_materialized_batch(output, batch_size))
79-
}
80-
AggregateHashTableState::Done => Ok(None),
81-
AggregateHashTableState::Building(_) => {
82-
internal_err!("next_output_batch must be called in the outputting state")
83-
}
84-
}
85-
}
86-
87-
fn materialize_final_output(
88-
&self,
89-
mut state: AggregateHashTableBuffer,
90-
output_schema: SchemaRef,
91-
) -> Result<MaterializedAggregateOutput> {
92-
// Final aggregate evaluation consumes accumulator state. Evaluate all
93-
// groups once, then slice the materialized batch on subsequent polls.
94-
let emit_to = EmitTo::All;
95-
let timer = self.group_by_metrics.emitting_time.timer();
96-
let mut output = state.group_values.emit(emit_to)?;
97-
98-
for acc in state.accumulators.iter_mut() {
99-
output.push(acc.evaluate(emit_to)?);
100-
}
101-
drop(timer);
102-
103-
let batch = RecordBatch::try_new(output_schema, output)?;
104-
debug_assert!(batch.num_rows() > 0);
105-
Ok(MaterializedAggregateOutput::new(batch))
106-
}
107-
108-
fn emit_next_materialized_batch(
109-
&mut self,
110-
mut output: MaterializedAggregateOutput,
111-
batch_size: usize,
112-
) -> Option<RecordBatch> {
113-
let batch = output.next_batch(batch_size);
114-
if output.is_exhausted() {
115-
self.state = AggregateHashTableState::Done;
116-
} else {
117-
self.state = AggregateHashTableState::OutputtingMaterialized(output);
118-
}
119-
batch
58+
self.next_output_batch_inner(HashAggregateAccumulator::evaluate_to_columns)
12059
}
12160

61+
/// Final aggregation consumes partial aggregate states and merges them into
62+
/// the table's partial-state accumulators.
12263
pub(in crate::aggregates) fn aggregate_batch(
12364
&mut self,
12465
batch: &RecordBatch,
12566
) -> Result<()> {
126-
let evaluated_batch = self.evaluate_batch(batch)?;
127-
let state = self.state.building_mut();
128-
129-
let timer = self.group_by_metrics.aggregation_time.timer();
130-
for group_values in &evaluated_batch.grouping_set_args {
131-
state
132-
.group_values
133-
.intern(group_values, &mut state.batch_group_indices)?;
134-
let group_indices = &state.batch_group_indices;
135-
let total_num_groups = state.group_values.len();
136-
137-
for (acc, values) in state
138-
.accumulators
139-
.iter_mut()
140-
.zip(evaluated_batch.accumulator_args.iter())
141-
{
142-
acc.merge_batch(values, group_indices, total_num_groups)?;
143-
}
144-
}
145-
drop(timer);
146-
147-
Ok(())
67+
self.aggregate_batch_inner(batch, HashAggregateAccumulator::merge_batch)
14868
}
14969

15070
pub(in crate::aggregates) fn start_output(&mut self) -> Result<()> {

datafusion/physical-plan/src/aggregates/aggregate_hash_table/partial_reduce_table.rs

Lines changed: 6 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,13 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18-
use std::sync::Arc;
19-
2018
use arrow::datatypes::SchemaRef;
2119
use arrow::record_batch::RecordBatch;
22-
use datafusion_common::{Result, internal_err};
23-
use datafusion_expr::EmitTo;
20+
use datafusion_common::Result;
2421

2522
use crate::aggregates::AggregateExec;
2623

27-
use super::common::{
28-
AggregateHashTable, AggregateHashTableBuffer, AggregateHashTableState,
29-
MaterializedAggregateOutput, PartialReduceMarker,
30-
};
24+
use super::common::{AggregateHashTable, HashAggregateAccumulator, PartialReduceMarker};
3125

3226
/// Methods specific to the aggregate hash table used in the partial-reduce stage.
3327
impl AggregateHashTable<PartialReduceMarker> {
@@ -55,91 +49,16 @@ impl AggregateHashTable<PartialReduceMarker> {
5549
pub(in crate::aggregates) fn next_output_batch(
5650
&mut self,
5751
) -> Result<Option<RecordBatch>> {
58-
let output_schema = Arc::clone(&self.output_schema);
59-
let batch_size = self.batch_size;
60-
// Take ownership of the output state. Note `emit_next_materialized_batch`
61-
// updates state after it emits a materialized slice.
62-
match std::mem::replace(&mut self.state, AggregateHashTableState::Done) {
63-
AggregateHashTableState::Outputting(state) => {
64-
if state.group_values.is_empty() {
65-
return Ok(None);
66-
}
67-
68-
let output =
69-
self.materialize_partial_reduce_output(state, output_schema)?;
70-
Ok(self.emit_next_materialized_batch(output, batch_size))
71-
}
72-
AggregateHashTableState::OutputtingMaterialized(output) => {
73-
Ok(self.emit_next_materialized_batch(output, batch_size))
74-
}
75-
AggregateHashTableState::Done => Ok(None),
76-
AggregateHashTableState::Building(_) => {
77-
internal_err!("next_output_batch must be called in the outputting state")
78-
}
79-
}
80-
}
81-
82-
fn materialize_partial_reduce_output(
83-
&self,
84-
mut state: AggregateHashTableBuffer,
85-
output_schema: SchemaRef,
86-
) -> Result<MaterializedAggregateOutput> {
87-
// `state(EmitTo::All)` consumes accumulator state. Emit all groups once,
88-
// then slice the materialized batch on subsequent polls.
89-
let emit_to_all = EmitTo::All;
90-
let timer = self.group_by_metrics.emitting_time.timer();
91-
let mut output = state.group_values.emit(emit_to_all)?;
92-
93-
for acc in state.accumulators.iter_mut() {
94-
output.extend(acc.state(emit_to_all)?);
95-
}
96-
drop(timer);
97-
98-
let batch = RecordBatch::try_new(output_schema, output)?;
99-
debug_assert!(batch.num_rows() > 0);
100-
Ok(MaterializedAggregateOutput::new(batch))
101-
}
102-
103-
fn emit_next_materialized_batch(
104-
&mut self,
105-
mut output: MaterializedAggregateOutput,
106-
batch_size: usize,
107-
) -> Option<RecordBatch> {
108-
let batch = output.next_batch(batch_size);
109-
if output.is_exhausted() {
110-
self.state = AggregateHashTableState::Done;
111-
} else {
112-
self.state = AggregateHashTableState::OutputtingMaterialized(output);
113-
}
114-
batch
52+
self.next_output_batch_inner(HashAggregateAccumulator::state)
11553
}
11654

55+
/// Partial-reduce aggregation consumes partial aggregate states and merges
56+
/// them into the table's partial-state accumulators.
11757
pub(in crate::aggregates) fn aggregate_batch(
11858
&mut self,
11959
batch: &RecordBatch,
12060
) -> Result<()> {
121-
let evaluated_batch = self.evaluate_batch(batch)?;
122-
let state = self.state.building_mut();
123-
124-
let timer = self.group_by_metrics.aggregation_time.timer();
125-
for group_values in &evaluated_batch.grouping_set_args {
126-
state
127-
.group_values
128-
.intern(group_values, &mut state.batch_group_indices)?;
129-
let group_indices = &state.batch_group_indices;
130-
let total_num_groups = state.group_values.len();
131-
132-
for (acc, values) in state
133-
.accumulators
134-
.iter_mut()
135-
.zip(evaluated_batch.accumulator_args.iter())
136-
{
137-
acc.merge_batch(values, group_indices, total_num_groups)?;
138-
}
139-
}
140-
drop(timer);
141-
142-
Ok(())
61+
self.aggregate_batch_inner(batch, HashAggregateAccumulator::merge_batch)
14362
}
14463

14564
pub(in crate::aggregates) fn start_output(&mut self) -> Result<()> {

0 commit comments

Comments
 (0)