zhuqi-lucas commented on code in PR #8146:
URL: https://github.com/apache/arrow-rs/pull/8146#discussion_r2280417779
##########
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 when large batch coming, and flush the buffer and
directly return to completed without copying to in progress.
>
> So i think we already no copy here?
--
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
