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

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

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



##########
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:
       Another thing we should consider (in a separate PR) is determining upfront which colums are left/rigth and avoid calling `schema.index_of` for each column in each batch. It is a small cost but we could do it once upfront.




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