2010YOUY01 commented on code in PR #23657:
URL: https://github.com/apache/datafusion/pull/23657#discussion_r3638095484
##########
datafusion/physical-plan/src/aggregates/ordered_partial_stream.rs:
##########
@@ -69,14 +70,50 @@ use crate::{InputOrderMode, RecordBatchStream,
SendableRecordBatchStream, metric
/// `k = 100`, it is safe to emit all groups with keys less than 100 because
the
/// input is ordered.
///
+/// # Memory Pressure and Spilling
+///
+/// ## Fully ordered case
+///
+/// If the input is ordered by every group key, for example:
+///
+/// - Input order: `a, b`
+/// - `GROUP BY`: `a, b`
+///
+/// Completed groups can be emitted as soon as the next group is observed.
Thus,
+/// only the current group remains active after completed groups are emitted,
and
+/// memory usage does not grow with the total number of groups.
+///
+/// If a memory reservation nevertheless fails, the stream returns the error
+/// directly, indicating an unexpected behavior.
+///
+/// ## Partially ordered case
+///
+/// If the input is ordered by only a subset of the group keys, for example:
+///
+/// - Input order: `a`
+/// - `GROUP BY`: `a, b`
+///
+/// If one `a` value contains many distinct `b` values, the table may
accumulate
+/// enough groups to exceed the memory limit.
+///
+/// - `OrderedPartialAggregateStream`: On reservation failure, it emits all
current
+/// intermediate states downstream and resets the table. The final stage can
+/// merge repeated `(a, b)` state rows, so no disk spill is required.
+/// - `OrderedFinalAggregateStream`: It cannot emit incomplete final results.
On
+/// reservation failure, it sorts the current intermediate states by the
complete
+/// group key and spills them as one run. After the input ends, it spills any
+/// remaining states, performs a sort-preserving merge of all runs, and
feeds the
+/// merged input into a fully ordered final aggregate stream.
+///
/// ## Implementation Note
///
/// This is intentionally kept simple and closely maps to
/// `GroupedHashAggregateStream` to finish the refactor sooner.
///
+/// More applicable optimizations are left to future work.
Review Comment:
Removed. That was just what I was thinking when implementing it, there is no
existing issue for it, I'll try to explore it later.
--
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]
