alamb commented on code in PR #9050:
URL: https://github.com/apache/arrow-datafusion/pull/9050#discussion_r1470254179
##########
datafusion/optimizer/src/push_down_filter.rs:
##########
@@ -519,28 +497,19 @@ fn push_down_all_join(
// it always will be the last element, otherwise result
// vector will contain only join keys (without additional
// element representing filter).
- let expr = join_plan.expressions();
- let mut new_exprs = if !on_filter_empty {
- expr[..expr.len() - 1].to_vec()
- } else {
- expr
- };
- if !join_conditions.is_empty() {
- new_exprs.push(join_conditions.into_iter().reduce(Expr::and).unwrap());
+ let mut exprs = join_plan.expressions();
Review Comment:
that is certainly a nice cleanup too 👍
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -590,13 +590,13 @@ impl LogicalPlan {
pub fn with_new_exprs(
&self,
mut expr: Vec<Expr>,
- inputs: &[LogicalPlan],
+ mut inputs: Vec<LogicalPlan>,
) -> Result<LogicalPlan> {
match self {
// Since expr may be different than the previous expr, schema of
the projection
// may change. We need to use try_new method instead of
try_new_with_schema method.
LogicalPlan::Projection(Projection { .. }) => {
- Projection::try_new(expr, Arc::new(inputs[0].clone()))
+ Projection::try_new(expr, Arc::new(inputs.swap_remove(0)))
Review Comment:
I think `swap_remove` on a one element vector is basically the same as
`pop().unwrap()`
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -752,8 +752,8 @@ impl LogicalPlan {
}).collect::<Result<Vec<(Expr, Expr)>>>()?;
Ok(LogicalPlan::Join(Join {
- left: Arc::new(inputs[0].clone()),
- right: Arc::new(inputs[1].clone()),
+ left: Arc::new(inputs.swap_remove(0)),
Review Comment:
I think this is correct, but relies on there being exactly 2 inputs which
seems like a reasonable assumption at this point
--
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]
