alamb commented on a change in pull request #660:
URL: https://github.com/apache/arrow-datafusion/pull/660#discussion_r664699757
##########
File path: datafusion/tests/sql.rs
##########
@@ -1694,6 +1694,17 @@ async fn equijoin() -> Result<()> {
Ok(())
}
+#[tokio::test]
+async fn equijoin_unsuppored_condition() -> Result<()> {
Review comment:
this test appears to be mis-named now. Perhaps something like
```suggestion
async fn equijoin_and_other_condition() -> Result<()> {
```
##########
File path: datafusion/src/sql/planner.rs
##########
@@ -368,15 +368,34 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
// parse ON expression
let expr = self.sql_to_rex(sql_expr, &join_schema)?;
+ // expression that didn't match equi-join pattern
+ let mut filter = vec![];
+
// extract join keys
- extract_join_keys(&expr, &mut keys)?;
+ extract_join_keys(&expr, &mut keys, &mut filter);
let (left_keys, right_keys): (Vec<Column>, Vec<Column>) =
keys.into_iter().unzip();
// return the logical plan representing the join
- LogicalPlanBuilder::from(left)
- .join(right, join_type, left_keys, right_keys)?
+ let join = LogicalPlanBuilder::from(left)
+ .join(right, join_type, left_keys, right_keys)?;
+
+ if filter.is_empty() {
+ join.build()
+ } else if join_type == JoinType::Inner {
+ join.filter(
Review comment:
this is a fancy way of creating an `AND` chain 👍
##########
File path: datafusion/src/sql/planner.rs
##########
@@ -368,15 +368,34 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
// parse ON expression
let expr = self.sql_to_rex(sql_expr, &join_schema)?;
+ // expression that didn't match equi-join pattern
+ let mut filter = vec![];
+
// extract join keys
- extract_join_keys(&expr, &mut keys)?;
+ extract_join_keys(&expr, &mut keys, &mut filter);
let (left_keys, right_keys): (Vec<Column>, Vec<Column>) =
keys.into_iter().unzip();
// return the logical plan representing the join
- LogicalPlanBuilder::from(left)
- .join(right, join_type, left_keys, right_keys)?
+ let join = LogicalPlanBuilder::from(left)
+ .join(right, join_type, left_keys, right_keys)?;
+
+ if filter.is_empty() {
+ join.build()
+ } else if join_type == JoinType::Inner {
Review comment:
👍 this is the right behavior.
--
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]
