niebayes commented on code in PR #15055:
URL: https://github.com/apache/datafusion/pull/15055#discussion_r1984349654


##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -906,28 +906,43 @@ impl LogicalPlan {
                 let equi_expr_count = on.len();
                 assert!(expr.len() >= equi_expr_count);
 
+                let col_pair_count =
+                    expr.iter().filter(|e| matches!(e, 
Expr::Column(_))).count() / 2;
+
                 // Assume that the last expr, if any,
                 // is the filter_expr (non equality predicate from ON clause)
-                let filter_expr = if expr.len() > equi_expr_count {
+                let filter_expr = if expr.len() - col_pair_count > 
equi_expr_count {
                     expr.pop()
                 } else {
                     None
                 };
 
                 // The first part of expr is equi-exprs,
                 // and the struct of each equi-expr is like `left-expr = 
right-expr`.
-                assert_eq!(expr.len(), equi_expr_count);
-                let new_on = expr.into_iter().map(|equi_expr| {
+                assert_eq!(expr.len() - col_pair_count, equi_expr_count);
+                let mut new_on = Vec::new();
+                let mut iter = expr.into_iter();
+                while let Some(equi_expr) = iter.next() {
                     // SimplifyExpression rule may add alias to the equi_expr.
                     let unalias_expr = equi_expr.clone().unalias();
-                    if let Expr::BinaryExpr(BinaryExpr { left, op: 
Operator::Eq, right }) = unalias_expr {
-                        Ok((*left, *right))
-                    } else {
-                        internal_err!(
-                            "The front part expressions should be an binary 
equality expression, actual:{equi_expr}"
-                        )
+                    match unalias_expr {
+                        Expr::BinaryExpr(BinaryExpr {
+                            left,
+                            op: Operator::Eq,
+                            right,
+                        }) => new_on.push((*left, *right)),
+                        left @ Expr::Column(_) => {
+                            let Some(right) = iter.next() else {
+                                internal_err!("Expected a pair of columns to 
construct the join on expression")?
+                            };
+
+                            new_on.push((left, right));
+                        }
+                        _ => internal_err!(
+                            "The front part expressions should be a binary 
equality expression or a column expression, actual:{equi_expr}"

Review Comment:
   ```suggestion
                               "The front part expressions should be a binary 
equality expression or a column expression, actual: {equi_expr}"
   ```



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to