jayzhan211 commented on issue #25183: URL: https://github.com/apache/datafusion/issues/25183#issuecomment-5716086007
I think we should keep and document the current behavior rather than expand it. The overshoot is bounded to one output batch per spill loop and is released when the write completes. Fixed-width types are already covered by the merge's 2x-largest-batch reservation, so pre-sizing them buys little. The types that actually overshoot (Utf8/Binary/View/nested) need per-row size prediction inside the merge, which is the hard part and not yet covered. The cost side (smaller batches, more writer calls) is certain. Keeping the best-effort fallback for the single-row case is right; failing there is the regression we removed from #24923. Suggested policy to document: the limit is enforced at reservation time; on the spill path we never fail a query for a shortfall that failing would not reclaim, and transient overshoot is bounded by one output batch per stream. If someone revisits the fuller design later, it should handle variable-width types and be measured on sort_tpch under a memory limit that forces multi-pass merging, plus a Utf8-heavy variant. -- 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]
