2010YOUY01 commented on code in PR #23181:
URL: https://github.com/apache/datafusion/pull/23181#discussion_r3504375797
##########
datafusion/core/tests/fuzz_cases/aggregate_fuzz.rs:
##########
@@ -373,9 +378,6 @@ async fn run_aggregate_test(input1: Vec<RecordBatch>,
group_by_columns: Vec<&str
.await
.unwrap();
assert!(collected_running.len() > 2);
- // Running should produce more chunk than the usual AggregateExec.
- // Otherwise it means that we cannot generate result in running mode.
- assert!(collected_running.len() > collected_usual.len());
Review Comment:
This makes sense with the current implementation.
However, in the future, we might want to accumulate `batch_size` groups
before emitting, so downstream operators can be better vectorized. In that
case, this assertion would no longer hold.
So we would need a different assertion to check whether the ordered
aggregation variant is used, likely via `EXPLAIN`. However, let's delay that
change until this idea is actually implemented.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
