zanmato1984 commented on code in PR #41122:
URL: https://github.com/apache/arrow/pull/41122#discussion_r1561207979


##########
cpp/src/arrow/acero/swiss_join.cc:
##########
@@ -2160,6 +2160,9 @@ Status JoinResidualFilter::FilterOneBatch(const 
ExecBatch& keypayload_batch,
                                           bool output_key_ids, bool 
output_payload_ids,
                                           arrow::util::TempVectorStack* 
temp_stack,
                                           int* num_passing_rows) const {
+  if (num_batch_rows == 0) {

Review Comment:
   Maybe it's better to move this shortcut below the line `*num_passing_rows = 
0;` for the following two considerations:
   1. The bunch of `ARROW_DCHECK` at the top of this function is doing basic 
logical checks thus we'd better do them all the time regardless of the shortcut.
   2. This function doesn't assume the out parameter `num_passing_rows` has a 
proper initial value, so we should always assign it a meaningful one.



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

Reply via email to