rluvaton commented on issue #24704:
URL: https://github.com/apache/datafusion/issues/24704#issuecomment-5468310397

   @alamb Thank you for opening this issue!
   
   Also, couple of things to do right away, is marking the `GroupValues` and 
the implementations of it as private, the reason they are public from what I 
checked is to allow doing micro benchmarks.
   
   ------
   
   My branch `add-blocks-impl` currently goes the easy-ish way, what if we 
could do as many breaking changes as we want, and we only need to support 
blocked approach, the reason I took that path is that:
   1. I want to see what it would take
   2. Is the blocked approach easily implemented by existing accumulators
   3. To be able to run benchmark on the best case scenario 
   4. `EmitTo::All`/`EmitTo::First` and emit blocks does not work together very 
well if the data structure is built in blocks:
        Keeping `EmitTo::All` means that if we emit the output as a single 
array it requires concatenation which I don't like
        Keeping  `EmitTo::First` means that blocks are now not fixed size so 
indexing is impossible without extra expensive computation.
   
   -----
   
   A problem I have with not having `EmitTo::First` is that think of this 
scenario.
   the block size is 1000, we can emit early (because the input is ordered for 
example)
   and we got all the values in the first 500 groups, now we got 2 more batches 
for group 501, and when we try to get another batch we don't have enough 
memory, if we don't have `EmitTo::First`, we can't emit the first 500 finished 
groups, we can emit the whole block which in our case is partial and group 501 
is not finished, which I don't have solution for this yet.
   
   


-- 
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]

Reply via email to