avantgardnerio commented on code in PR #3334:
URL: https://github.com/apache/arrow-datafusion/pull/3334#discussion_r961118110


##########
datafusion/sql/src/planner.rs:
##########
@@ -2453,6 +2453,17 @@ fn remove_join_expressions(
                     (_, Some(rr)) => Ok(Some(rr)),
                     _ => Ok(None),
                 }
+            },
+            // Fix for issue#78 join predicates from inside of OR expr also 
pulled up properly.
+            Operator::Or => {

Review Comment:
   I don't think we can pull out `or`s the same way we would part of an `and` - 
altering a disjunction will affect the outcome. i.e. if it was 
`l.partkey=s.partkey or l.price < 5.00` and we extract the first part into a 
join clause, we will now only see ones with prices < 5, not ones that matched 
or were less than 5. A contrived example I'm sure, but I think I'd have to see 
a counter-example working before I am confident we didn't alter the results.
   
   I think a better place to solve this might be with an optimizer rule, 
instead of in the planner - in an optimizer rule we could always just return 
`Err()` if the situation isn't perfectly to our liking.



-- 
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...@arrow.apache.org

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

Reply via email to