yashaggarwal85d commented on code in PR #26637:
URL: https://github.com/apache/datafusion/pull/26637#discussion_r4379376853
##########
datafusion/physical-plan/src/aggregates/clustered_final_stream.rs:
##########
@@ -732,4 +749,34 @@ mod tests {
assert_eq!(pool.reserved(), 0);
Ok(())
}
+
+ #[tokio::test]
+ async fn fully_clustered_completed_groups_under_memory_pressure() ->
Result<()> {
+ let (plan, mode) =
+ sum_over_sorted_input(AggregateMode::Final, &["k"],
full_batches())?;
+ assert_eq!(mode, GroupClusteringMode::Full);
+ assert_completed_groups_emitted_under_limit(&plan).await
+ }
+
+ /// A single key never completes a group before the input ends. The limit
+ /// fits only the initial table.
+ #[tokio::test]
+ async fn memory_pressure_without_completed_groups() -> Result<()> {
+ let (plan, _) =
+ sum_over_sorted_input(AggregateMode::Final, &["k"],
single_key_batches())?;
+ let limit = held_memory(&plan).await?;
+ assert_resources_exhausted_under_limit(&plan, limit).await
+ }
+
+ /// Without temporary files, partial group clustering takes the same path
as
+ /// full clustering. Its group values keep their capacity after emitting,
so
+ /// one byte below the unlimited footprint still fails.
+ #[tokio::test]
+ async fn partially_clustered_memory_pressure_without_spilling() ->
Result<()> {
Review Comment:
Fair point. It passes on main too, so it doesn't really test the fix. I
removed both copies and `partial_batches` (c3ea1ce). I'll open a separate issue
for the retained capacity.
--
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]
