[GitHub] [arrow] Dandandan commented on a change in pull request #9048: ARROW-11076: [Rust][DataFusion] Refactor usage of right indices in hash join

[GitBox <gi...@apache.org>]

Dandandan commented on a change in pull request #9048:
URL: https://github.com/apache/arrow/pull/9048#discussion_r550260158



##########
File path: rust/datafusion/src/physical_plan/hash_join.rs
##########
@@ -276,55 +276,58 @@ fn build_batch_from_indices(
         todo!("Create empty record batch");
     }
     // this is just for symmetry of the code below.
-    let right = vec![right.clone()];
+    let right_batches = vec![right.clone()];
 
     let (primary_is_left, primary, secondary) = match join_type {
-        JoinType::Inner | JoinType::Left => (true, left, &right),
-        JoinType::Right => (false, &right, left),
+        JoinType::Inner | JoinType::Left => (true, left, &right_batches),
+        JoinType::Right => (false, &right_batches, left),
     };
 
     // build the columns of the new [RecordBatch]:
     // 1. pick whether the column is from the left or right
     // 2. based on the pick, `take` items from the different recordBatches
     let mut columns: Vec<Arc<dyn Array>> = Vec::with_capacity(schema.fields().len());
+
+    let right_indices = indices.iter().map(|(_, join_index)| join_index).collect();
+
     for field in schema.fields() {
         // pick the column (left or right) based on the field name.
-        // Note that we take `.data_ref()` to gather the [ArrayData] of each array.
-        let (is_primary, arrays) = match primary[0].schema().index_of(field.name()) {
-            Ok(i) => Ok((true, primary.iter().map(|batch| batch.column(i).data_ref().as_ref()).collect::<Vec<_>>())),
+        let (is_primary, column_index) = match primary[0].schema().index_of(field.name()) {

Review comment:
       Yes, agreed, anything we can move out of this inner loop is an improvement, and avoids slowdowns, e.g. with large nr. of columns / smaller batch sizes.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org