jonathanc-n commented on code in PR #16380: URL: https://github.com/apache/datafusion/pull/16380#discussion_r2141169183
########## datafusion/physical-plan/src/joins/hash_join.rs: ########## @@ -1372,15 +1407,16 @@ pub fn equal_rows_arr( // The results are then folded (combined) using the and function to get a final equality result. equal = iter .map(|(left, right)| { - let arr_left = take(left.as_ref(), indices_left, None)?; + let left_refs: Vec<&dyn Array> = left.iter().map(Arc::as_ref).collect(); + let arr_left = interleave(&left_refs, indices_left)?; let arr_right = take(right.as_ref(), indices_right, None)?; eq_dyn_null(arr_left.as_ref(), arr_right.as_ref(), null_equals_null) }) .try_fold(equal, |acc, equal2| and(&acc, &equal2?))?; let filter_builder = FilterBuilder::new(&equal).optimize().build(); - let left_filtered = filter_builder.filter(indices_left)?; + let left_filtered = filter_builder.filter(indices_left)?; // annoying Review Comment: ```let mut left_filtered = Vec::with_capacity(equal.len()); for (i, &pair) in indices_left.iter().enumerate() { if equal.value(i) { left_filtered.push(pair); } }``` I currently did this for now, but it defeats the whole point of the mask ########## datafusion/physical-plan/src/joins/hash_join.rs: ########## @@ -1372,15 +1407,16 @@ pub fn equal_rows_arr( // The results are then folded (combined) using the and function to get a final equality result. equal = iter .map(|(left, right)| { - let arr_left = take(left.as_ref(), indices_left, None)?; + let left_refs: Vec<&dyn Array> = left.iter().map(Arc::as_ref).collect(); + let arr_left = interleave(&left_refs, indices_left)?; let arr_right = take(right.as_ref(), indices_right, None)?; eq_dyn_null(arr_left.as_ref(), arr_right.as_ref(), null_equals_null) }) .try_fold(equal, |acc, equal2| and(&acc, &equal2?))?; let filter_builder = FilterBuilder::new(&equal).optimize().build(); - let left_filtered = filter_builder.filter(indices_left)?; + let left_filtered = filter_builder.filter(indices_left)?; // annoying Review Comment: ```let mut left_filtered = Vec::with_capacity(equal.len()); for (i, &pair) in indices_left.iter().enumerate() { if equal.value(i) { left_filtered.push(pair); } } ``` I currently did this for now, but it defeats the whole point of the mask -- 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