zhuqi-lucas commented on code in PR #8146:
URL: https://github.com/apache/arrow-rs/pull/8146#discussion_r2280417701
##########
arrow-select/src/coalesce.rs:
##########
@@ -236,6 +262,13 @@ impl BatchCoalescer {
/// assert_eq!(completed_batch, expected_batch);
/// ```
pub fn push_batch(&mut self, batch: RecordBatch) -> Result<(), ArrowError>
{
+ if let Some(limit) = self.biggest_coalesce_batch_size {
Review Comment:
Thank you @Dandandan and @alamb for review!
> I wonder if this makes only sense whenever there are no in progress
batches? So whenever we get a large batch and we do not need to copy, just
output the batch, in other cases go on with the default path.
> I don't fully understand this comment -- I think the code in this PR is a
noop when there are no in progress batches (because finish_buffered_batch will
not do anything)
Yeah, actually here we already avoid the in progress batches operation
because we will return early and flush the buffer and directly return to
completed without copying to in progress.
So i think we already no copy here? I am not sure i am missing something,
please correct me, thanks!
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
