comphead commented on code in PR #14815:
URL: https://github.com/apache/datafusion/pull/14815#discussion_r1966358685


##########
datafusion-cli/src/exec.rs:
##########
@@ -269,6 +269,11 @@ pub(super) async fn exec_and_print(
                         
reservation.try_grow(get_record_batch_memory_size(&batch))?;
                         results.push(batch);
                     }
+                } else if let MaxRows::Unlimited = print_options.maxrows {

Review Comment:
   That is a good catch, wondering to avoid code duplication would it be better 
to
   
   ```
                   let max_rows = match print_options.maxrows {
                       MaxRows::Unlimited => usize::MAX,
                       MaxRows::Limited(n) => n
                   };
   
              while let Some(batch) = stream.next().await {
                   let batch = batch?;
                   let curr_num_rows = batch.num_rows();
                   if row_count < max_rows + curr_num_rows {
                           // Try to grow the reservation to accommodate the 
batch in memory
                           
reservation.try_grow(get_record_batch_memory_size(&batch))?;
                           results.push(batch);
                       }
                   row_count += curr_num_rows;
               }
   ```
   
   so we also remove unnecessary branch and literal evaulation every batch



-- 
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

Reply via email to