Kristofferjoha opened a new pull request, #27360:
URL: https://github.com/apache/datafusion/pull/27360

   ## Which issue does this PR close?
   
   - Part of #27323. The merge retaining every input batch and the repartition 
over-counting shared buffers are still open.
   
   ## Rationale for this change
   
   A streaming GROUP BY with a Utf8View column among several group columns 
emits small batches that carry far more string data than their rows use. The 
cause is in `ByteViewGroupValueBuilder::take_n`. On each partial emit it copies 
the in-progress string buffer from byte 0 up to the last emitted string, and 
never trims that buffer. So every emitted batch carries the strings of all 
earlier batches from the same block.
   
   ## What changes are included in this PR?
   
   After a partial take from the in-progress buffer, drain the taken bytes from 
it and shift the offsets of the remaining views. Each emitted batch then holds 
only its own strings. `drain` moves the not-yet-emitted bytes to the front of 
the buffer on each partial emit. In the streaming case that is only a few 
groups.
   
   Not changed: when the last emitted string is in a completed block, the 
emitted batch still shares that whole block (up to 2 MB). In the streaming case 
that should now be rare, since the in-progress buffer only holds groups not yet 
emitted.
   
   ## What is the testing strategy for this PR?
   
   New unit test `test_byte_view_take_n_drains_in_progress`. It takes one row 
at a time and checks that each emitted array's data buffer holds only its own 
string, that the in-progress buffer shrinks, and that the remaining and newly 
appended values are still correct.
   
   I also ran the repro from the issue with the cli, and compared to main:
   
   | Memory limit | `main` | This PR |
   | --- | --- | --- |
   | `-m 200M` | ResourcesExhausted | passes |
   | `-m 50M` | ResourcesExhausted | passes |
   | `-m 20M` | ResourcesExhausted | passes |
   | `-m 10M` | ResourcesExhausted | passes |
   
   `EXPLAIN ANALYZE` of the same query without a memory limit:
   
   | Operator | `main` output_bytes | This PR output_bytes |
   | --- | --- | --- |
   | `AggregateExec` (Partial) | 42.6 MB | 1023.1 KB |
   | `RepartitionExec` | 330.0 MB | 5.5 MB |
   
   ## Are there any user-facing changes?
   
   No


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