yonipeleg33 commented on PR #48468: URL: https://github.com/apache/arrow/pull/48468#issuecomment-3891168582
> @yonipeleg33 Thanks for your explains, so you did not set a default value to make it backwards-compatible IIUC. > > > [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. > > I mentioned the recursion because I find your implementation will first check max_rows, and try to split current batch to two sub-batches to write, in the split you did not need check max_bytes because it will be checked in the next recursion. > > But in this patch, we will determine the batch_size based on both max_rows and max_bytes together. > so you did not set a default value to make it backwards-compatible IIUC. Exactly. > But in this patch, we will determine the batch_size based on both max_rows and max_bytes together. I understand, and I think setting max_bytes to INT_MAX should produce a practically identical behaviour as if it was unset, in the loop case - because the loop splits based on a logical OR between the two limits, so in practice the byte limit will always be < INT_MAX. -- 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]
