Skip to content

Commit d112ecc

Browse files
committed
refactor: separate aggregate group completion from input ordering
1 parent c3ef346 commit d112ecc

46 files changed

Lines changed: 1081 additions & 843 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎datafusion/core/tests/dataframe/mod.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3422,9 +3422,9 @@ async fn union_with_mix_of_presorted_and_explicitly_resorted_inputs_with_reparti
34223422
assert_snapshot!(
34233423
union_with_mix_of_presorted_and_explicitly_resorted_inputs_impl(false).await?,
34243424
@r"
3425-
AggregateExec: mode=Final, gby=[id@0 as id], aggr=[], ordering_mode=Sorted
3425+
AggregateExec: mode=Final, gby=[id@0 as id], aggr=[], group_completion_mode=Full
34263426
SortPreservingMergeExec: [id@0 ASC NULLS LAST]
3427-
AggregateExec: mode=Partial, gby=[id@0 as id], aggr=[], ordering_mode=Sorted
3427+
AggregateExec: mode=Partial, gby=[id@0 as id], aggr=[], group_completion_mode=Full
34283428
UnionExec
34293429
DataSourceExec: file_groups={1 group: [[{testdata}/alltypes_tiny_pages.parquet]]}, projection=[id], output_ordering=[id@0 ASC NULLS LAST], file_type=parquet
34303430
SortExec: expr=[id@0 ASC NULLS LAST], preserve_partitioning=[false]
@@ -3440,9 +3440,9 @@ async fn union_with_mix_of_presorted_and_explicitly_resorted_inputs_with_reparti
34403440
assert_snapshot!(
34413441
union_with_mix_of_presorted_and_explicitly_resorted_inputs_impl(true).await?,
34423442
@r"
3443-
AggregateExec: mode=Final, gby=[id@0 as id], aggr=[], ordering_mode=Sorted
3443+
AggregateExec: mode=Final, gby=[id@0 as id], aggr=[], group_completion_mode=Full
34443444
SortPreservingMergeExec: [id@0 ASC NULLS LAST]
3445-
AggregateExec: mode=Partial, gby=[id@0 as id], aggr=[], ordering_mode=Sorted
3445+
AggregateExec: mode=Partial, gby=[id@0 as id], aggr=[], group_completion_mode=Full
34463446
UnionExec
34473447
DataSourceExec: file_groups={1 group: [[{testdata}/alltypes_tiny_pages.parquet]]}, projection=[id], output_ordering=[id@0 ASC NULLS LAST], file_type=parquet
34483448
SortExec: expr=[id@0 ASC NULLS LAST], preserve_partitioning=[false]

‎datafusion/core/tests/fuzz_cases/aggregate_fuzz.rs‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,14 @@ use datafusion_common_runtime::JoinSet;
4141
use datafusion_functions_aggregate::sum::sum_udaf;
4242
use datafusion_physical_expr::PhysicalSortExpr;
4343
use datafusion_physical_expr::expressions::{Column, col, lit};
44-
use datafusion_physical_plan::InputOrderMode;
4544
use test_utils::{StringBatchGenerator, add_empty_batches};
4645

4746
use datafusion_execution::TaskContext;
4847
use datafusion_execution::memory_pool::FairSpillPool;
4948
use datafusion_execution::runtime_env::RuntimeEnvBuilder;
5049
use datafusion_physical_expr::aggregate::AggregateExprBuilder;
5150
use datafusion_physical_plan::aggregates::{
52-
AggregateExec, AggregateMode, PhysicalGroupBy,
51+
AggregateExec, AggregateMode, GroupCompletionMode, PhysicalGroupBy,
5352
};
5453
use datafusion_physical_plan::metrics::MetricValue;
5554
use datafusion_physical_plan::{ExecutionPlan, collect, displayable};
@@ -303,7 +302,7 @@ async fn streaming_aggregate_test() {
303302
/// two `AggregateExec` variants produce the same result: the pipeline breaking
304303
/// one over unordered input (`PartialHashAggregateStream`) and the
305304
/// non-pipeline breaking one over ordered input
306-
/// (`OrderedPartialAggregateStream`).
305+
/// (`ClusteredPartialAggregateStream`).
307306
async fn run_aggregate_test(input1: Vec<RecordBatch>, group_by_columns: Vec<&str>) {
308307
let schema = input1[0].schema();
309308
let session_config = SessionConfig::new().with_batch_size(50);
@@ -354,8 +353,8 @@ async fn run_aggregate_test(input1: Vec<RecordBatch>, group_by_columns: Vec<&str
354353
.unwrap(),
355354
);
356355
assert_ne!(
357-
aggregate_exec_running.input_order_mode(),
358-
&InputOrderMode::Linear,
356+
aggregate_exec_running.group_completion_mode(),
357+
&GroupCompletionMode::None,
359358
"running aggregate should observe ordered input for group_by: {group_by:?}"
360359
);
361360

@@ -555,13 +554,17 @@ async fn verify_ordered_aggregate(frame: &DataFrame, expected_sort: bool) {
555554

556555
fn f_down(&mut self, node: &'n Self::Node) -> Result<TreeNodeRecursion> {
557556
if let Some(exec) = node.downcast_ref::<AggregateExec>() {
557+
assert_eq!(
558+
exec.properties().output_ordering().is_some(),
559+
self.expected_sort
560+
);
558561
if self.expected_sort {
559562
assert!(matches!(
560-
exec.input_order_mode(),
561-
InputOrderMode::PartiallySorted(_) | InputOrderMode::Sorted
563+
exec.group_completion_mode(),
564+
GroupCompletionMode::Partial(_) | GroupCompletionMode::Full
562565
));
563566
} else {
564-
assert_eq!(*exec.input_order_mode(), InputOrderMode::Linear);
567+
assert_eq!(*exec.group_completion_mode(), GroupCompletionMode::None);
565568
}
566569
}
567570
Ok(TreeNodeRecursion::Continue)

‎datafusion/core/tests/fuzz_cases/aggregation_fuzzer/query_builder.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,9 @@ pub struct QueryBuilder {
9393
/// ...
9494
/// ```
9595
///
96-
/// More details can see [`GroupOrdering`].
96+
/// More details can see [`GroupCompletion`].
9797
///
98-
/// [`GroupOrdering`]: datafusion_physical_plan::aggregates::order::GroupOrdering
98+
/// [`GroupCompletion`]: datafusion_physical_plan::aggregates::order::GroupCompletion
9999
dataset_sort_keys: Vec<Vec<String>>,
100100

101101
/// If we will also test the no grouping case like:

‎datafusion/core/tests/physical_optimizer/enforce_distribution.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4867,9 +4867,9 @@ fn preserve_ordering_for_streaming_sorted_aggregate() -> Result<()> {
48674867

48684868
let plan_distrib = test_config.to_plan(physical_plan.clone(), &DISTRIB_DISTRIB_SORT);
48694869
assert_plan!(plan_distrib, @r"
4870-
AggregateExec: mode=FinalPartitioned, gby=[a@0 as a], aggr=[COUNT(b)], ordering_mode=Sorted
4870+
AggregateExec: mode=FinalPartitioned, gby=[a@0 as a], aggr=[COUNT(b)], group_completion_mode=Full
48714871
RepartitionExec: partitioning=Hash([a@0], 2), input_partitions=2, preserve_order=true, sort_exprs=a@0 ASC
4872-
AggregateExec: mode=Partial, gby=[a@0 as a], aggr=[COUNT(b)], ordering_mode=Sorted
4872+
AggregateExec: mode=Partial, gby=[a@0 as a], aggr=[COUNT(b)], group_completion_mode=Full
48734873
DataSourceExec: file_groups={2 groups: [[x], [y]]}, projection=[a, b, c, d, e], output_ordering=[a@0 ASC], file_type=parquet
48744874
");
48754875

@@ -4901,9 +4901,9 @@ fn preserve_ordering_for_streaming_partially_sorted_aggregate() -> Result<()> {
49014901

49024902
let plan_distrib = test_config.to_plan(physical_plan.clone(), &DISTRIB_DISTRIB_SORT);
49034903
assert_plan!(plan_distrib, @r"
4904-
AggregateExec: mode=FinalPartitioned, gby=[a@0 as a, b@1 as b], aggr=[COUNT(c)], ordering_mode=PartiallySorted([0])
4904+
AggregateExec: mode=FinalPartitioned, gby=[a@0 as a, b@1 as b], aggr=[COUNT(c)], group_completion_mode=Partial([0])
49054905
RepartitionExec: partitioning=Hash([a@0, b@1], 2), input_partitions=2, preserve_order=true, sort_exprs=a@0 ASC
4906-
AggregateExec: mode=Partial, gby=[a@0 as a, b@1 as b], aggr=[COUNT(c)], ordering_mode=PartiallySorted([0])
4906+
AggregateExec: mode=Partial, gby=[a@0 as a, b@1 as b], aggr=[COUNT(c)], group_completion_mode=Partial([0])
49074907
DataSourceExec: file_groups={2 groups: [[x], [y]]}, projection=[a, b, c, d, e], output_ordering=[a@0 ASC], file_type=parquet
49084908
");
49094909

‎datafusion/core/tests/physical_optimizer/limited_distinct_aggregation.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ fn test_has_order_by() -> Result<()> {
520520
actual,
521521
@r"
522522
LocalLimitExec: fetch=10
523-
AggregateExec: mode=Single, gby=[a@0 as a], aggr=[], ordering_mode=Sorted
523+
AggregateExec: mode=Single, gby=[a@0 as a], aggr=[], group_completion_mode=Full
524524
DataSourceExec: file_groups={1 group: [[x]]}, projection=[a, b, c, d, e], output_ordering=[a@0 ASC], file_type=parquet
525525
"
526526
);

‎datafusion/core/tests/physical_optimizer/pushdown_sort.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -731,13 +731,13 @@ fn test_pushdown_through_blocking_node() {
731731
OptimizationTest:
732732
input:
733733
- SortExec: expr=[a@0 ASC], preserve_partitioning=[false]
734-
- AggregateExec: mode=Final, gby=[a@0 as a], aggr=[COUNT(b)], ordering_mode=Sorted
734+
- AggregateExec: mode=Final, gby=[a@0 as a], aggr=[COUNT(b)], group_completion_mode=Full
735735
- SortExec: expr=[a@0 DESC NULLS LAST], preserve_partitioning=[false]
736736
- DataSourceExec: file_groups={1 group: [[x]]}, projection=[a, b, c, d, e], output_ordering=[a@0 ASC], file_type=parquet
737737
output:
738738
Ok:
739739
- SortExec: expr=[a@0 ASC], preserve_partitioning=[false]
740-
- AggregateExec: mode=Final, gby=[a@0 as a], aggr=[COUNT(b)], ordering_mode=Sorted
740+
- AggregateExec: mode=Final, gby=[a@0 as a], aggr=[COUNT(b)], group_completion_mode=Full
741741
- SortExec: expr=[a@0 DESC NULLS LAST], preserve_partitioning=[false]
742742
- DataSourceExec: file_groups={1 group: [[x]]}, projection=[a, b, c, d, e], file_type=parquet, sort_order_for_reorder=[a@0 DESC NULLS LAST], reverse_row_groups=true
743743
"

‎datafusion/physical-plan/benches/dictionary_group_values.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use criterion::{
2929
};
3030
use datafusion_expr::EmitTo;
3131
use datafusion_physical_plan::aggregates::group_values::new_group_values;
32-
use datafusion_physical_plan::aggregates::order::{GroupOrdering, GroupOrderingFull};
32+
use datafusion_physical_plan::aggregates::order::{GroupCompletion, GroupCompletionFull};
3333
use rand::rngs::StdRng;
3434
use rand::seq::SliceRandom;
3535
use rand::{Rng, SeedableRng};
@@ -106,7 +106,7 @@ fn bench_intern_emit(c: &mut Criterion) {
106106
b.iter_batched_ref(
107107
|| {
108108
(
109-
new_group_values(schema.clone(), &GroupOrdering::None)
109+
new_group_values(schema.clone(), &GroupCompletion::None)
110110
.unwrap(),
111111
Vec::<usize>::with_capacity(size),
112112
)
@@ -151,7 +151,7 @@ fn bench_repeated_intern_emit(c: &mut Criterion) {
151151
b.iter_batched_ref(
152152
|| {
153153
(
154-
new_group_values(schema.clone(), &GroupOrdering::None)
154+
new_group_values(schema.clone(), &GroupCompletion::None)
155155
.unwrap(),
156156
Vec::<usize>::with_capacity(size),
157157
)
@@ -172,7 +172,7 @@ fn bench_repeated_intern_emit(c: &mut Criterion) {
172172
group.finish();
173173
}
174174

175-
// GroupOrdering::Full -> GroupValuesColumn::<true>: scalar append_val/equal_to path.
175+
// GroupCompletion::Full -> GroupValuesColumn::<true>: scalar append_val/equal_to path.
176176
fn bench_scalar_append_equal(c: &mut Criterion) {
177177
let mut group = c.benchmark_group("dict_scalar_append_equal");
178178
let schema = dict_schema();
@@ -192,7 +192,7 @@ fn bench_scalar_append_equal(c: &mut Criterion) {
192192
(
193193
new_group_values(
194194
schema.clone(),
195-
&GroupOrdering::Full(GroupOrderingFull::new()),
195+
&GroupCompletion::Full(GroupCompletionFull::new()),
196196
)
197197
.unwrap(),
198198
Vec::<usize>::with_capacity(size),
@@ -227,7 +227,7 @@ fn bench_take_n(c: &mut Criterion) {
227227
b.iter_batched_ref(
228228
|| {
229229
(
230-
new_group_values(schema.clone(), &GroupOrdering::None).unwrap(),
230+
new_group_values(schema.clone(), &GroupCompletion::None).unwrap(),
231231
Vec::<usize>::with_capacity(size),
232232
)
233233
},
@@ -298,7 +298,7 @@ fn bench_shared_values_arc(c: &mut Criterion) {
298298
b.iter_batched_ref(
299299
|| {
300300
(
301-
new_group_values(schema.clone(), &GroupOrdering::None)
301+
new_group_values(schema.clone(), &GroupCompletion::None)
302302
.unwrap(),
303303
Vec::<usize>::with_capacity(size),
304304
)

‎datafusion/physical-plan/benches/ordered_group_values.rs‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@ use datafusion_physical_expr::expressions::col;
3838
use datafusion_physical_expr::{LexOrdering, PhysicalSortExpr};
3939
use datafusion_physical_plan::aggregates::group_values::multi_group_by::GroupValuesColumn;
4040
use datafusion_physical_plan::aggregates::group_values::{GroupValues, new_group_values};
41-
use datafusion_physical_plan::aggregates::order::GroupOrdering;
41+
use datafusion_physical_plan::aggregates::order::GroupCompletion;
4242
use datafusion_physical_plan::aggregates::{
43-
AggregateExec, AggregateMode, PhysicalGroupBy,
43+
AggregateExec, AggregateMode, GroupCompletionMode, PhysicalGroupBy,
4444
};
4545
use datafusion_physical_plan::test::TestMemoryExec;
46-
use datafusion_physical_plan::{ExecutionPlan, InputOrderMode, collect};
46+
use datafusion_physical_plan::{ExecutionPlan, collect};
4747
use tokio::runtime::Runtime;
4848

4949
const ROWS: usize = 131_072;
@@ -112,8 +112,10 @@ fn grouping(c: &mut Criterion) {
112112
let values: Box<dyn GroupValues> = if selected {
113113
new_group_values(
114114
Arc::clone(&schema),
115-
&GroupOrdering::try_new(&InputOrderMode::Sorted)
116-
.unwrap(),
115+
&GroupCompletion::try_new(
116+
&GroupCompletionMode::Full,
117+
)
118+
.unwrap(),
117119
)
118120
.unwrap()
119121
} else {
@@ -158,7 +160,7 @@ fn check_case(schema: &SchemaRef, batches: &[Vec<ArrayRef>]) -> (usize, usize) {
158160
let mut hashed = GroupValuesColumn::<true>::try_new(Arc::clone(schema)).unwrap();
159161
let mut selected = new_group_values(
160162
Arc::clone(schema),
161-
&GroupOrdering::try_new(&InputOrderMode::Sorted).unwrap(),
163+
&GroupCompletion::try_new(&GroupCompletionMode::Full).unwrap(),
162164
)
163165
.unwrap();
164166
let mut expected = Vec::new();
@@ -188,7 +190,7 @@ fn aggregate_plan(
188190
schema: &SchemaRef,
189191
keys: Vec<Vec<ArrayRef>>,
190192
sort_columns: &[&str],
191-
input_order_mode: &InputOrderMode,
193+
group_completion_mode: &GroupCompletionMode,
192194
) -> Arc<dyn ExecutionPlan> {
193195
let mut fields = schema.fields().to_vec();
194196
fields.push(Arc::new(Field::new("v", DataType::Int64, false)));
@@ -232,7 +234,7 @@ fn aggregate_plan(
232234
schema,
233235
)
234236
.unwrap();
235-
assert_eq!(plan.input_order_mode(), input_order_mode);
237+
assert_eq!(plan.group_completion_mode(), group_completion_mode);
236238
Arc::new(plan)
237239
}
238240

@@ -246,7 +248,7 @@ fn aggregation(c: &mut Criterion) {
246248
for run_length in [1, 8, 128, 8192] {
247249
let (schema, keys) = inputs(run_length, 8192, strings);
248250
let plan =
249-
aggregate_plan(&schema, keys, &["a", "b"], &InputOrderMode::Sorted);
251+
aggregate_plan(&schema, keys, &["a", "b"], &GroupCompletionMode::Full);
250252
let name =
251253
format!("{}_run{run_length}", if strings { "string" } else { "int" });
252254
group.bench_function(name, |b| {
@@ -291,7 +293,7 @@ fn partially_ordered_aggregation(c: &mut Criterion) {
291293
&schema,
292294
keys,
293295
&["a"],
294-
&InputOrderMode::PartiallySorted(vec![0]),
296+
&GroupCompletionMode::Partial(vec![0]),
295297
);
296298
let runtime = Runtime::new().unwrap();
297299
let mut group = c.benchmark_group("partially_ordered_aggregate_exec");

‎datafusion/physical-plan/benches/partial_ordering.rs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
use std::sync::Arc;
1919

2020
use arrow::array::{ArrayRef, Int32Array};
21-
use datafusion_physical_plan::aggregates::order::GroupOrderingPartial;
21+
use datafusion_physical_plan::aggregates::order::GroupCompletionPartial;
2222

2323
use criterion::{Criterion, criterion_group, criterion_main};
2424

@@ -34,20 +34,20 @@ fn create_test_arrays(num_columns: usize) -> Vec<ArrayRef> {
3434
.collect()
3535
}
3636
fn bench_new_groups(c: &mut Criterion) {
37-
let mut group = c.benchmark_group("group_ordering_partial");
37+
let mut group = c.benchmark_group("group_completion_partial");
3838

39-
// Test with 1, 2, 4, and 8 order indices
39+
// Test with 1, 2, 4, and 8 grouping indices
4040
for num_columns in [1, 2, 4, 8] {
41-
let order_indices: Vec<usize> = (0..num_columns).collect();
41+
let grouping_indices: Vec<usize> = (0..num_columns).collect();
4242

43-
group.bench_function(format!("order_indices_{num_columns}"), |b| {
43+
group.bench_function(format!("grouping_indices_{num_columns}"), |b| {
4444
let batch_group_values = create_test_arrays(num_columns);
4545
let group_indices: Vec<usize> = (0..BATCH_SIZE).collect();
4646

4747
b.iter(|| {
48-
let mut ordering =
49-
GroupOrderingPartial::try_new(order_indices.clone()).unwrap();
50-
ordering
48+
let mut completion =
49+
GroupCompletionPartial::try_new(grouping_indices.clone()).unwrap();
50+
completion
5151
.new_groups(&batch_group_values, &group_indices, BATCH_SIZE)
5252
.unwrap();
5353
});

0 commit comments

Comments
 (0)