[GitHub] [arrow-datafusion] houqp commented on a change in pull request #1566: fix: sql planner creates cross join instead of inner join from select predicates

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

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

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