alamb commented on issue #24704: URL: https://github.com/apache/datafusion/issues/24704#issuecomment-5681390310
> I'm not talking about file backed mmap. I don't understand why would the existence of disk is needed? Sorry I misunderstood your proposal > We cannot use the existing GroupsAccumulator trait > Given that we still want to have the EmitTo::All we can't use the existing trait since emitting all in blocked aggregate Should output Vec of Blocks rather than 1 very large block. I don't understand this assertion. I think in previous versions @Rachelint basically changed the internal implementations of accumulators (to be something like Vec<...>) and then just used the existing `EmitTo::First(n)` API. The key idea was that you knew the size of the `n` (it is `batch_size`) so the accumulators could size their internal blocks to that output size. If there was a mismatch (e.g. for versions that can emit early) then the accumulators did something less efficient (like copying / `concat`ing batches together) > Having EmitTo::First(n) for the BlockedGroupsAccumulator is recommended The usecase as I understand it is for aggregation streams that need to emit early (e.g. they are partially sorted on their group keys). If we don't have EmitTo::First(n) we need some other way to emit all but the most recent group I think > Most of the blocked aggregate expressions and group by types need a small number of building blocks This is good news > Don't force blocked helpers to pre-allocate block_size Makes sense (though I think the idea that we limit the blocks to block_size makes sense0 -- 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]
