rluvaton commented on code in PR #25877:
URL: https://github.com/apache/datafusion/pull/25877#discussion_r4145080217
##########
datafusion/physical-plan/src/aggregates/spill.rs:
##########
@@ -248,6 +248,22 @@ impl AggregateSpill {
Ok(())
}
+ /// [`Self::sort_and_spill`] for the state batches of a hash aggregate
+ /// table: one batch for flat storage, one per block for blocked storage,
+ /// each written as its own spill file.
+ pub(super) fn sort_and_spill_batches(
+ &mut self,
+ state_batches: Vec<MaterializedBatch>,
+ ) -> Result<()> {
+ // TODO: sort across multiple arrays without concat (see #24928
Review Comment:
I did not want to increase memory usage by doing concat,
consider this scenerio: we have 10M rows where the cardinality is very high
(1-2 items per group) we are doing group by on 130 columns, and only have sum
aggregate expression.
this will concat 130 columns (which can also cause i32 offsets error for
byte arrays types) and have large tmp memory where we reach here when we don't
have enough memory for **another batch** to be accumulated.
Also for large memory I would prefer to reserve memory for this sort since
we might not have enough for it (even though this is temporary computation that
is not being held).
we should maybe consider using `ExternalSorter` here as it already handle
spilling, sort in memory, etc.
--
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]
