Skip to content

Commit a077a27

Browse files
committed
Enhance MIN/MAX integration test and clean up macros
Update the MIN/MAX integration test in basic.rs to use two MemTable partitions, ensuring the physical plan includes both partial and final aggregate stages. Retain checks for dictionary-typed output schema and results. In min_max.rs, remove the no-op min_max_dictionary! macro and inline the existing generic comparison helper for improved clarity and efficiency.
1 parent 7127c20 commit a077a27

File tree

2 files changed

+29
-19
lines changed

2 files changed

+29
-19
lines changed

datafusion/core/tests/sql/aggregates/basic.rs

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use super::*;
1919
use datafusion::common::test_util::batches_to_string;
2020
use datafusion_catalog::MemTable;
2121
use datafusion_common::ScalarValue;
22+
use datafusion_physical_plan::displayable;
2223
use insta::assert_snapshot;
2324

2425
#[tokio::test]
@@ -444,23 +445,40 @@ async fn count_distinct_dictionary_mixed_values() -> Result<()> {
444445

445446
#[tokio::test]
446447
async fn min_max_dictionary_uses_planned_dictionary_path() -> Result<()> {
447-
let ctx = SessionContext::new();
448-
449-
let dict_values = StringArray::from(vec!["a", "z", "zz_unused"]);
450-
let dict_indices = Int32Array::from(vec![Some(1), Some(1), None]);
451-
let dict = DictionaryArray::new(dict_indices, Arc::new(dict_values));
448+
let ctx = SessionContext::new_with_config(SessionConfig::new().with_target_partitions(2));
452449

453450
let dict_type =
454451
DataType::Dictionary(Box::new(DataType::Int32), Box::new(DataType::Utf8));
455452
let schema = Arc::new(Schema::new(vec![Field::new("dict", dict_type.clone(), true)]));
456453

457-
let batch = RecordBatch::try_new(schema.clone(), vec![Arc::new(dict)])?;
458-
let provider = MemTable::try_new(schema, vec![vec![batch]])?;
454+
let batch1 = RecordBatch::try_new(
455+
schema.clone(),
456+
vec![Arc::new(DictionaryArray::new(
457+
Int32Array::from(vec![Some(1), Some(1), None]),
458+
Arc::new(StringArray::from(vec!["a", "z", "zz_unused"])),
459+
))],
460+
)?;
461+
let batch2 = RecordBatch::try_new(
462+
schema.clone(),
463+
vec![Arc::new(DictionaryArray::new(
464+
Int32Array::from(vec![Some(0), Some(1)]),
465+
Arc::new(StringArray::from(vec!["a", "d"])),
466+
))],
467+
)?;
468+
let provider = MemTable::try_new(schema, vec![vec![batch1], vec![batch2]])?;
459469
ctx.register_table("t", Arc::new(provider))?;
460470

461471
let df = ctx
462472
.sql("SELECT min(dict) AS min_dict, max(dict) AS max_dict FROM t")
463473
.await?;
474+
let physical_plan = df.clone().create_physical_plan().await?;
475+
let formatted_plan = format!("{}", displayable(physical_plan.as_ref()).indent(true));
476+
assert!(formatted_plan.contains("AggregateExec: mode=Partial, gby=[]"));
477+
assert!(
478+
formatted_plan.contains("AggregateExec: mode=Final, gby=[]")
479+
|| formatted_plan.contains("AggregateExec: mode=FinalPartitioned, gby=[]")
480+
);
481+
464482
let results = df.collect().await?;
465483

466484
assert_eq!(results[0].schema().field(0).data_type(), &dict_type);
@@ -472,7 +490,7 @@ async fn min_max_dictionary_uses_planned_dictionary_path() -> Result<()> {
472490
+----------+----------+
473491
| min_dict | max_dict |
474492
+----------+----------+
475-
| z | z |
493+
| a | z |
476494
+----------+----------+
477495
"
478496
);

datafusion/functions-aggregate-common/src/min_max.rs

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,6 @@ macro_rules! min_max_generic {
141141
}};
142142
}
143143

144-
macro_rules! min_max_dictionary {
145-
($VALUE:expr, $DELTA:expr, $OP:ident) => {{ min_max_generic!($VALUE, $DELTA, $OP) }};
146-
}
147-
148144
// min/max of two scalar values of the same type
149145
macro_rules! min_max {
150146
($VALUE:expr, $DELTA:expr, $OP:ident) => {{
@@ -423,26 +419,22 @@ macro_rules! min_max {
423419
) => {
424420
wrap_dictionary_scalar(
425421
key_type.as_ref(),
426-
min_max_dictionary!(
427-
lhs_inner.as_ref(),
428-
rhs_inner.as_ref(),
429-
$OP
430-
),
422+
min_max_generic!(lhs_inner.as_ref(), rhs_inner.as_ref(), $OP),
431423
)
432424
}
433425

434426
(
435427
ScalarValue::Dictionary(_, lhs_inner),
436428
rhs,
437429
) => {
438-
min_max_dictionary!(lhs_inner.as_ref(), rhs, $OP)
430+
min_max_generic!(lhs_inner.as_ref(), rhs, $OP)
439431
}
440432

441433
(
442434
lhs,
443435
ScalarValue::Dictionary(_, rhs_inner),
444436
) => {
445-
min_max_dictionary!(lhs, rhs_inner.as_ref(), $OP)
437+
min_max_generic!(lhs, rhs_inner.as_ref(), $OP)
446438
}
447439

448440
e => {

0 commit comments

Comments
 (0)