houqp commented on a change in pull request #1566:
URL: https://github.com/apache/arrow-datafusion/pull/1566#discussion_r786524099
##########
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])
+ || idx == mut_plans.len() - 1
+ {
+ left = LogicalPlanBuilder::from(left)
+ .cross_join(&mut_plans[idx])?
+ .build()?;
+ } else {
+ mut_plans.push(mut_plans[idx].clone());
Review comment:
could be helpful to add a simple comment here to explain why pushing the
same plan to the end helps. my understanding is we want to retry the join key
matching after we go through all the plan once just in case this plan can be
indirectly joined with left through other plans.
##########
File path: datafusion/src/logical_plan/plan.rs
##########
@@ -214,8 +220,14 @@ pub struct Extension {
pub node: Arc<dyn UserDefinedLogicalNode + Send + Sync>,
}
+impl PartialEq for Extension {
+ fn eq(&self, _other: &Self) -> bool {
+ todo!()
Review comment:
What's the reason for implementing PartialEq for all the plan nodes?
--
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
