berkaysynnada commented on code in PR #11966:
URL: https://github.com/apache/datafusion/pull/11966#discussion_r1715744641
##########
datafusion/physical-plan/src/coalesce_batches.rs:
##########
@@ -364,90 +393,60 @@ impl BatchCoalescer {
Arc::clone(&self.schema)
}
- /// Add a batch, returning a batch if the target batch size or limit is
reached
- fn push_batch(&mut self, batch: RecordBatch) ->
Result<Option<RecordBatch>> {
- // discard empty batches
- if batch.num_rows() == 0 {
- return Ok(None);
- }
+ fn push_batch(&mut self, batch: RecordBatch) -> CoalescerState {
+ let batch = gc_string_view_batch(&batch);
Review Comment:
I don't have a deep knowledge about gc'ing the batch. If I move the gc()
after limit check, and if a batch leads to a limit-reached case, the lastly
added batch wouldn't be handled by gc(). This isn't a problem?
--
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...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
