adriangb commented on PR #25910: URL: https://github.com/apache/datafusion/pull/25910#issuecomment-5920256250
Thanks @andygrove! Addressed in 0bc3b6c and 7dec81f (description updated): 1. The merge of spill files now also caps output batches by the spill bound, estimated from average row size. Still not reserved in the pool, so skewed rows can exceed it (noted under limitations). 2. 0 now means no bound; the affected tests pass at their old speed. 3. Every written piece is compacted, including unsplit batches. 4. Compaction is recursive, and halving stops when it doesn't shrink pieces. 5. Bound is now `max(reservation / 4, 1 MiB)`. 6. Only dictionaries, view arrays and list views are copied; other columns stay zero-copy slices. Pieces are still compacted before the first write; happy to switch to one-at-a-time if you think it matters. 7. `with_max_batch_bytes` is now `pub(crate)`, documented as only for readers that read to the end. 8. ListView is now handled; dense Union and REE are still written whole (listed as a limitation). 9. Used dictionary values are measured with a bitmap, no GC. 10. Rows are counted per written piece. 11. The integration tests now assert that the sort spilled and that merged batches stay within the bound. Kept in Rust since sqllogictest can't check those. 12. `round_trip` now checks read-back sizes against `max_record_batch_memory`; stale comment fixed. Also switched to `total_buffer_bytes_used()` and rewrote the compaction docs. -- 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]
