houqp commented on a change in pull request #1566:
URL: https://github.com/apache/arrow-datafusion/pull/1566#discussion_r787300922
##########
File path: datafusion/src/sql/planner.rs
##########
@@ -731,20 +732,33 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
}
}
if join_keys.is_empty() {
- left =
-
LogicalPlanBuilder::from(left).cross_join(right)?.build()?;
+ // check [1..idx] if contains current plan to avoid
infinite loop
+ if mut_plans[1..idx].contains(&mut_plans[idx])
Review comment:
Got it, I am thinking that we might be able to avoid this extra
PartialEq and contains check overhead by breaking this up into two loops:
1. extract the loop logic into its own function
2. first loop through `plans` once, for each plan that doesn't have join key
matches, we push its index in `plans` into a retry vector
3. second loop to go through the retry vector
This way, we can guarantee every plan in the original `plans` vector is
evaluated at most twice without getting into an infinite loop while avoiding
particleq compare and clones. What do you think?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
