ygf11 commented on code in PR #4193:
URL: https://github.com/apache/arrow-datafusion/pull/4193#discussion_r1024918161
##########
datafusion/sql/src/planner.rs:
##########
@@ -750,9 +746,46 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
.unwrap_or(Ok(join))?
.build()
} else {
- LogicalPlanBuilder::from(left)
- .join(&right, join_type, (left_keys, right_keys),
join_filter)?
- .build()
+ // Wrap projection for left input if left join keys
contain normal expression.
+ let (left_child, left_projected) =
+ wrap_projection_for_join_if_necessary(&left_keys,
left)?;
+ let left_join_keys = left_keys
+ .iter()
+ .map(|key| {
+ key.try_into_col()
+ .or_else(|_|
Ok(Column::from_name(key.display_name()?)))
+ })
+ .collect::<Result<Vec<_>>>()?;
+
+ // Wrap projection for right input if right join keys
contains normal expression.
+ let (right_child, right_projected) =
+ wrap_projection_for_join_if_necessary(&right_keys,
right)?;
+ let right_join_keys = right_keys
+ .iter()
+ .map(|key| {
+ key.try_into_col()
+ .or_else(|_|
Ok(Column::from_name(key.display_name()?)))
+ })
+ .collect::<Result<Vec<_>>>()?;
+
+ let join_plan_builder =
LogicalPlanBuilder::from(left_child).join(
+ &right_child,
+ join_type,
+ (left_join_keys, right_join_keys),
+ join_filter,
+ )?;
+
+ // Remove temporary projected columns if necessary.
+ if left_projected || right_projected {
+ let final_join_result = join_schema
Review Comment:
Trim the temp projected columns here, and the test case is
`test_select_all_inner_join`.
cc @mingmwang
##########
datafusion/sql/src/planner.rs:
##########
@@ -3005,6 +3079,32 @@ fn extract_possible_join_keys(
}
}
+/// Wrap projection for a plan, if the join keys contains normal expression.
+fn wrap_projection_for_join_if_necessary(
+ join_keys: &[Expr],
+ input: LogicalPlan,
+) -> Result<(LogicalPlan, bool)> {
+ let expr_join_keys = join_keys
+ .iter()
+ .flat_map(|expr| expr.try_into_col().is_err().then_some(expr))
+ .cloned()
+ .collect::<HashSet<Expr>>();
Review Comment:
When projection items contain duplicated expressions, the sql will failed.
So I do deduplication here, relative test case is also added.
cc @alamb @mingmwang
--
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]
