[GitHub] [arrow-datafusion] alamb commented on a change in pull request #470: Support semi join

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

alamb commented on a change in pull request #470:
URL: https://github.com/apache/arrow-datafusion/pull/470#discussion_r644268005



##########
File path: datafusion/src/physical_plan/hash_join.rs
##########
@@ -1666,6 +1689,41 @@ mod tests {
         Ok(())
     }
 
+    #[tokio::test]
+    async fn join_semi() -> Result<()> {
+        let left = build_table(
+            ("a1", &vec![1, 2, 3]),
+            ("b1", &vec![4, 5, 7]), // 7 does not exist on the right
+            ("c1", &vec![7, 8, 9]),
+        );
+        let right = build_table(

Review comment:
       It may be worth testing with record batches that have duplicates on the `left` as well (to ensure the duplicates on the left are produced) ?

##########
File path: datafusion/src/physical_plan/hash_join.rs
##########
@@ -1109,22 +1116,34 @@ pub fn create_hashes<'a>(
 }
 
 // Produces a batch for left-side rows that are not marked as being visited during the whole join

Review comment:
       ```suggestion
   // Produces a batch for left-side rows that have/have not been matched during the whole join
   ```

##########
File path: datafusion/src/optimizer/hash_build_probe_order.rs
##########
@@ -187,6 +187,7 @@ impl OptimizerRule for HashBuildProbeOrder {
             | LogicalPlan::CreateExternalTable { .. }
             | LogicalPlan::Explain { .. }
             | LogicalPlan::Union { .. }
+            | LogicalPlan::Join { .. }

Review comment:
       should you also possibly conditionalize this on being a SemiJoin? like
   
   ```suggestion
               | LogicalPlan::Join if *join_type == JoinType::Semi { .. }
   ```
   
   So if we add a new join type in the future it doesn't accidentally get allowed / disallowed in some confusing way
   




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