yonipeleg33 commented on PR #48468: URL: https://github.com/apache/arrow/pull/48468#issuecomment-3890345822
> @alamb @yonipeleg33 [apache/arrow-rs#9357](https://github.com/apache/arrow-rs/pull/9357) is elegant with recursive, which makes it could handle max_rows and max_bytes individually. But if we change to use loop like arrow-cpp here, we need combine these two limits together in an iteration right? @wecharyu I haven't got into the code here too much (just taken a quick look), but I can explain my choice in the Rust PR: I wanted to be as backwards-compatible as possible, so in that case it means if the user doesn't provide a byte limit, the code should behave exactly the same as before this change. An I think this is possible to achieve in a loop as well, realistically: Set the byte limit to int's max value, and you practically never reach this code path (unless it's a _really_ extreme record batch). I just kept the recursion approach because it was already there before my change, so I kept the same code style. The important thing here is, IMO, to think of how the user is affected by any of these pesky conditions enforcing those limits. Hope this helps! -- 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]
