Skip to content

Commit 3a0ecb6

Browse files
committed
refactor: separate aggregate group completion from input ordering
1 parent 7ba0a9b commit 3a0ecb6

13 files changed

Lines changed: 208 additions & 111 deletions

‎datafusion/physical-plan/src/aggregates/aggregate_hash_table/common_ordered.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,12 @@ use datafusion_common::assert_or_internal_err;
2828
use datafusion_execution::memory_pool::proxy::VecAllocExt;
2929
use datafusion_expr::{AggregateMetrics, EmitTo};
3030

31-
use crate::InputOrderMode;
3231
use crate::PhysicalExpr;
3332
use crate::aggregates::group_values::{
3433
AccumulatorPhase, AggregateAccumulatorMetrics, AggregateArgumentMetrics,
3534
GroupByMetrics, GroupValues, new_group_values,
3635
};
37-
use crate::aggregates::order::GroupOrdering;
36+
use crate::aggregates::order::{GroupCompletionMode, GroupOrdering};
3837
use crate::aggregates::{
3938
AggregateExec, AggregateMode, PhysicalGroupBy, aggregate_expressions,
4039
evaluate_group_by,
@@ -115,7 +114,7 @@ impl OrderedAggregateTableMetrics {
115114
/// `OrderedAggrMode` selects the aggregate semantics. For example,
116115
/// `OrderedAggregateTable::<PartialMarker>::new(...)` consumes raw rows
117116
/// and emits partial states, while
118-
/// `OrderedAggregateTable::<FinalMarker>::new_with_input_order(...)`
117+
/// `OrderedAggregateTable::<FinalMarker>::new_with_group_completion(...)`
119118
/// consumes partial states and emits final values.
120119
///
121120
/// Shared methods live on `impl<T>`; single/partial/final behavior lives on
@@ -189,7 +188,7 @@ impl<AggrMode> OrderedAggregateTable<AggrMode> {
189188
output_schema: SchemaRef,
190189
state_schema: SchemaRef,
191190
batch_size: usize,
192-
input_order_mode: &InputOrderMode,
191+
group_completion_mode: &GroupCompletionMode,
193192
aggregate_mode: &AggregateMode,
194193
filters: Vec<Option<Arc<dyn PhysicalExpr>>>,
195194
metrics: OrderedAggregateTableMetrics,
@@ -199,7 +198,8 @@ impl<AggrMode> OrderedAggregateTable<AggrMode> {
199198
"OrderedAggregateTable requires config batch_size >= 1"
200199
);
201200

202-
let group_ordering = GroupOrdering::try_new(input_order_mode)?;
201+
let group_ordering =
202+
GroupOrdering::try_new_for_group_completion(group_completion_mode)?;
203203
let group_schema = agg.group_by.group_schema(input_schema)?;
204204
let group_values = new_group_values(group_schema, &group_ordering)?;
205205
let aggregate_arguments = aggregate_expressions(

‎datafusion/physical-plan/src/aggregates/aggregate_hash_table/ordered_final_table.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ use arrow::datatypes::SchemaRef;
2525
use arrow::record_batch::RecordBatch;
2626
use datafusion_common::Result;
2727

28-
use crate::InputOrderMode;
2928
use crate::aggregates::aggregate_hash_table::FinalMarker;
29+
use crate::aggregates::order::GroupCompletionMode;
3030
use crate::aggregates::{AggregateExec, AggregateMode, group_values::AccumulatorPhase};
3131

3232
use super::common::HashAggregateAccumulator;
@@ -42,12 +42,12 @@ use super::common_ordered::{OrderedAggregateTable, OrderedAggregateTableMetrics}
4242
///
4343
/// See comments at [`OrderedAggregateTable`] for details.
4444
impl OrderedAggregateTable<FinalMarker> {
45-
pub(in crate::aggregates) fn new_with_input_order(
45+
pub(in crate::aggregates) fn new_with_group_completion(
4646
agg: &AggregateExec,
4747
input_schema: &SchemaRef,
4848
output_schema: SchemaRef,
4949
batch_size: usize,
50-
input_order_mode: &InputOrderMode,
50+
group_completion_mode: &GroupCompletionMode,
5151
metrics: OrderedAggregateTableMetrics,
5252
) -> Result<Self> {
5353
Self::new_for_mode(
@@ -56,7 +56,7 @@ impl OrderedAggregateTable<FinalMarker> {
5656
output_schema,
5757
Arc::clone(input_schema),
5858
batch_size,
59-
input_order_mode,
59+
group_completion_mode,
6060
&AggregateMode::Final,
6161
vec![None; agg.aggr_expr.len()],
6262
metrics,

‎datafusion/physical-plan/src/aggregates/aggregate_hash_table/ordered_partial_table.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ impl OrderedAggregateTable<PartialMarker> {
6868
output_schema,
6969
state_schema,
7070
batch_size,
71-
&agg.input_order_mode,
71+
&agg.group_completion_mode,
7272
&AggregateMode::Partial,
7373
agg.filter_expr.iter().cloned().collect(),
7474
metrics,

‎datafusion/physical-plan/src/aggregates/aggregate_hash_table/ordered_single_table.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ impl OrderedAggregateTable<SingleMarker> {
5959
output_schema,
6060
state_schema,
6161
batch_size,
62-
&agg.input_order_mode,
62+
&agg.group_completion_mode,
6363
&agg.mode,
6464
agg.filter_expr.iter().cloned().collect(),
6565
metrics,

‎datafusion/physical-plan/src/aggregates/grouped_hash_stream.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,8 @@ impl GroupedHashAggregateStream {
528528
.collect::<Vec<_>>()
529529
.join(", ");
530530
let name = format!("GroupedHashAggregateStream[{partition}] ({agg_fn_names})");
531-
let group_ordering = GroupOrdering::try_new(&agg.input_order_mode)?;
531+
let group_ordering =
532+
GroupOrdering::try_new_for_group_completion(&agg.group_completion_mode)?;
532533
let oom_mode = match (agg.mode, &group_ordering) {
533534
// In partial aggregation mode, always prefer to emit incomplete results early.
534535
(AggregateMode::Partial, _) => OutOfMemoryMode::EmitEarly,

‎datafusion/physical-plan/src/aggregates/hash_stream.rs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,14 @@ use super::aggregate_hash_table::{
3737
AggregateHashTable, FinalMarker, OrderedAggregateTableMetrics, PartialMarker,
3838
PartialSkipMarker,
3939
};
40+
use super::order::GroupCompletionMode;
4041
use super::skip_partial::SkipAggregationProbe;
4142
use super::spill::AggregateSpill;
4243
use crate::metrics::{
4344
BaselineMetrics, MetricBuilder, MetricCategory, RecordOutput, SpillMetrics,
4445
};
4546
use crate::stream::{EmptyRecordBatchStream, RecordBatchStreamAdapter};
46-
use crate::{InputOrderMode, SendableRecordBatchStream, metrics};
47+
use crate::{SendableRecordBatchStream, metrics};
4748

4849
/// Hash aggregation is implemented in two stages: partial and final. This
4950
/// stream implements the partial stage.
@@ -217,7 +218,7 @@ impl PartialHashAggregateStream {
217218
partition: usize,
218219
) -> Result<Self> {
219220
debug_assert_eq!(agg.mode, super::AggregateMode::Partial);
220-
debug_assert_eq!(agg.input_order_mode, InputOrderMode::Linear);
221+
debug_assert_eq!(agg.group_completion_mode, GroupCompletionMode::None);
221222

222223
let schema = Arc::clone(&agg.schema);
223224
let input = agg.input.execute(partition, Arc::clone(context))?;
@@ -570,7 +571,7 @@ impl FinalHashAggregateStream {
570571
agg.mode,
571572
super::AggregateMode::Final | super::AggregateMode::FinalPartitioned
572573
));
573-
debug_assert_eq!(agg.input_order_mode, InputOrderMode::Linear);
574+
debug_assert_eq!(agg.group_completion_mode, GroupCompletionMode::None);
574575

575576
let input = agg.input.execute(partition, Arc::clone(context))?;
576577
Self::new_with_input(agg, context, partition, input)
@@ -604,7 +605,7 @@ impl FinalHashAggregateStream {
604605
context,
605606
partition,
606607
batch_size,
607-
&InputOrderMode::Linear,
608+
&GroupCompletionMode::None,
608609
&input_schema,
609610
spill_metrics,
610611
)?))
@@ -873,6 +874,7 @@ mod tests {
873874
use std::time::Duration;
874875

875876
use super::*;
877+
use crate::InputOrderMode;
876878
use crate::aggregates::{AggregateMode, PhysicalGroupBy};
877879
use crate::common::collect;
878880
use crate::execution_plan::ExecutionPlan;

‎datafusion/physical-plan/src/aggregates/mod.rs‎

Lines changed: 50 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ use datafusion_physical_expr_common::sort_expr::{
206206
use datafusion_expr::utils::AggregateOrderSensitivity;
207207
use datafusion_physical_expr_common::utils::evaluate_expressions_to_arrays;
208208
use itertools::Itertools;
209+
use order::GroupCompletionMode;
209210
use topk::hash_table::is_supported_hash_key_type;
210211
use topk::heap::is_supported_heap_type;
211212

@@ -895,6 +896,12 @@ pub struct AggregateExec {
895896
/// input order". When that is not possible, the constructor overwrites it
896897
/// with the unordered variant [`InputOrderMode::Linear`].
897898
input_order_mode: InputOrderMode,
899+
/// Describes when the executor can determine that groups are complete.
900+
///
901+
/// Input ordering describes a subset of the cases in which groups can be
902+
/// safely emitted before the input ends. Full group completion requires only
903+
/// that rows for each complete grouping tuple are contiguous.
904+
group_completion_mode: GroupCompletionMode,
898905
cache: Arc<PlanProperties>,
899906
/// During initialization, if the plan supports dynamic filtering (see [`AggrDynFilter`]),
900907
/// it is set to `Some(..)` regardless of whether it can be pushed down to a child node.
@@ -919,6 +926,7 @@ impl AggregateExec {
919926
required_input_ordering: self.required_input_ordering.clone(),
920927
metrics: ExecutionPlanMetricsSet::new(),
921928
input_order_mode: self.input_order_mode.clone(),
929+
group_completion_mode: self.group_completion_mode.clone(),
922930
cache: Arc::clone(&self.cache),
923931
mode: self.mode,
924932
group_by: Arc::clone(&self.group_by),
@@ -939,6 +947,7 @@ impl AggregateExec {
939947
required_input_ordering: self.required_input_ordering.clone(),
940948
metrics: ExecutionPlanMetricsSet::new(),
941949
input_order_mode: self.input_order_mode.clone(),
950+
group_completion_mode: self.group_completion_mode.clone(),
942951
cache: Arc::clone(&self.cache),
943952
mode: self.mode,
944953
group_by: Arc::clone(&self.group_by),
@@ -1072,6 +1081,8 @@ impl AggregateExec {
10721081
input_order_mode = InputOrderMode::Linear;
10731082
}
10741083

1084+
let group_completion_mode = GroupCompletionMode::from(&input_order_mode);
1085+
10751086
// construct a map from the input expression to the output expression of the Aggregation group by
10761087
let group_expr_mapping =
10771088
ProjectionMapping::try_new(group_by.expr.clone(), &input.schema())?;
@@ -1102,6 +1113,7 @@ impl AggregateExec {
11021113
required_input_ordering,
11031114
limit_options: None,
11041115
input_order_mode,
1116+
group_completion_mode,
11051117
cache: Arc::new(cache),
11061118
dynamic_filter: None,
11071119
};
@@ -1284,47 +1296,51 @@ impl AggregateExec {
12841296
);
12851297
}
12861298

1287-
// Choose the execution path based on (aggregation mode, ordering).
1288-
//
1289-
// Note that `self.input_order_mode` represents both input ordering and output
1290-
// order promise. See its comment for details.
1299+
// Choose the execution path based on aggregation mode and when groups
1300+
// are known to be complete.
12911301
use AggregateMode::*;
1292-
use InputOrderMode::*;
1293-
let stream = match (self.mode, &self.input_order_mode) {
1294-
(Partial, Linear) => StreamType::PartialHash(
1302+
let stream = match (self.mode, &self.group_completion_mode) {
1303+
(Partial, GroupCompletionMode::None) => StreamType::PartialHash(
12951304
PartialHashAggregateStream::new(self, context, partition)?,
12961305
),
1297-
(Partial, Sorted | PartiallySorted(_)) => {
1306+
(Partial, GroupCompletionMode::Partial(_) | GroupCompletionMode::Full) => {
12981307
StreamType::OrderedPartialAggregate(OrderedPartialAggregateStream::new(
12991308
self, context, partition,
13001309
)?)
13011310
}
1302-
(PartialReduce, Linear) => StreamType::PartialReduceHash(
1311+
(PartialReduce, GroupCompletionMode::None) => StreamType::PartialReduceHash(
13031312
PartialReduceHashAggregateStream::new(self, context, partition)?,
13041313
),
1305-
(PartialReduce, Sorted | PartiallySorted(_)) => {
1306-
// See the comment above: the builder enforces `Linear` order for
1307-
// `PartialReduce` mode.
1314+
(
1315+
PartialReduce,
1316+
GroupCompletionMode::Partial(_) | GroupCompletionMode::Full,
1317+
) => {
13081318
return internal_err!(
1309-
"PartialReduce aggregation must use InputOrderMode::Linear"
1319+
"PartialReduce aggregation must use GroupCompletionMode::None"
13101320
);
13111321
}
1312-
(Final | FinalPartitioned, Linear) => StreamType::FinalHash(
1313-
FinalHashAggregateStream::new(self, context, partition)?,
1314-
),
1315-
(Final | FinalPartitioned, Sorted | PartiallySorted(_)) => {
1316-
StreamType::OrderedFinalAggregate(OrderedFinalAggregateStream::new(
1322+
(Final | FinalPartitioned, GroupCompletionMode::None) => {
1323+
StreamType::FinalHash(FinalHashAggregateStream::new(
13171324
self, context, partition,
13181325
)?)
13191326
}
1320-
(Single | SinglePartitioned, Linear) => StreamType::SingleHash(
1321-
SingleHashAggregateStream::new(self, context, partition)?,
1322-
),
1323-
(Single | SinglePartitioned, Sorted | PartiallySorted(_)) => {
1324-
StreamType::OrderedSingleAggregate(OrderedSingleAggregateStream::new(
1327+
(
1328+
Final | FinalPartitioned,
1329+
GroupCompletionMode::Partial(_) | GroupCompletionMode::Full,
1330+
) => StreamType::OrderedFinalAggregate(OrderedFinalAggregateStream::new(
1331+
self, context, partition,
1332+
)?),
1333+
(Single | SinglePartitioned, GroupCompletionMode::None) => {
1334+
StreamType::SingleHash(SingleHashAggregateStream::new(
13251335
self, context, partition,
13261336
)?)
13271337
}
1338+
(
1339+
Single | SinglePartitioned,
1340+
GroupCompletionMode::Partial(_) | GroupCompletionMode::Full,
1341+
) => StreamType::OrderedSingleAggregate(OrderedSingleAggregateStream::new(
1342+
self, context, partition,
1343+
)?),
13281344
};
13291345
Ok(stream)
13301346
}
@@ -2352,6 +2368,8 @@ impl ExecutionPlan for AggregateExec {
23522368
required_input_ordering: _,
23532369
// Derived at construction from the input ordering and `group_by`.
23542370
input_order_mode: _,
2371+
// Derived at construction from `input_order_mode`.
2372+
group_completion_mode: _,
23552373
// Derived at construction by `Self::compute_properties`.
23562374
cache: _,
23572375
dynamic_filter,
@@ -5042,6 +5060,10 @@ mod tests {
50425060
aggregate.input_order_mode(),
50435061
InputOrderMode::PartiallySorted(_)
50445062
));
5063+
assert_eq!(
5064+
aggregate.group_completion_mode,
5065+
GroupCompletionMode::Partial(vec![0])
5066+
);
50455067

50465068
let task_ctx = new_migrated_hash_ctx(2);
50475069
let stream = aggregate.execute_typed(0, &task_ctx)?;
@@ -5491,6 +5513,7 @@ mod tests {
54915513
)?;
54925514

54935515
assert_eq!(aggregate.input_order_mode(), &InputOrderMode::Linear);
5516+
assert_eq!(aggregate.group_completion_mode, GroupCompletionMode::None);
54945517
// This captures the behavior before #24438. When the source can declare
54955518
// `(key, time_bin)` group-contiguous, the corresponding case can use
54965519
// `EmissionType::Incremental`.
@@ -5651,6 +5674,10 @@ mod tests {
56515674
Arc::clone(&schema),
56525675
)?;
56535676
assert_eq!(final_aggregate.input_order_mode(), &InputOrderMode::Sorted);
5677+
assert_eq!(
5678+
final_aggregate.group_completion_mode,
5679+
GroupCompletionMode::Full
5680+
);
56545681

56555682
let task_ctx = new_migrated_hash_ctx(2);
56565683
let stream = final_aggregate.execute_typed(0, &task_ctx)?;

0 commit comments

Comments
 (0)