From 494d747d028ab1fb210db14a475ab6693abf39b8 Mon Sep 17 00:00:00 2001 From: Raphael Taylor-Davies Date: Wed, 14 Dec 2022 22:05:13 +0000 Subject: [PATCH] Make LogicalPlanBuilder consuming (#4622) --- datafusion/core/src/dataframe.rs | 2 +- datafusion/expr/src/logical_plan/builder.rs | 150 +++++++++--------- datafusion/expr/src/utils.rs | 2 +- .../optimizer/src/common_subexpr_eliminate.rs | 2 +- .../optimizer/src/decorrelate_where_exists.rs | 2 +- .../optimizer/src/decorrelate_where_in.rs | 2 +- .../optimizer/src/eliminate_cross_join.rs | 64 ++++---- datafusion/optimizer/src/eliminate_limit.rs | 2 +- .../optimizer/src/eliminate_outer_join.rs | 10 +- .../optimizer/src/filter_null_join_keys.rs | 4 +- .../optimizer/src/propagate_empty_relation.rs | 4 +- datafusion/optimizer/src/push_down_filter.rs | 37 ++--- datafusion/optimizer/src/push_down_limit.rs | 30 ++-- .../optimizer/src/push_down_projection.rs | 6 +- .../optimizer/src/scalar_subquery_to_join.rs | 4 +- datafusion/proto/src/logical_plan.rs | 6 +- datafusion/sql/src/planner.rs | 33 ++-- 17 files changed, 181 insertions(+), 179 deletions(-) diff --git a/datafusion/core/src/dataframe.rs b/datafusion/core/src/dataframe.rs index 5e615be607c31..153c59c5648d6 100644 --- a/datafusion/core/src/dataframe.rs +++ b/datafusion/core/src/dataframe.rs @@ -351,7 +351,7 @@ impl DataFrame { ) -> Result { let plan = LogicalPlanBuilder::from(self.plan) .join( - &right.plan, + right.plan, join_type, (left_cols.to_vec(), right_cols.to_vec()), filter, diff --git a/datafusion/expr/src/logical_plan/builder.rs b/datafusion/expr/src/logical_plan/builder.rs index 1c3b813f70f71..8d873e5e4c6ae 100644 --- a/datafusion/expr/src/logical_plan/builder.rs +++ b/datafusion/expr/src/logical_plan/builder.rs @@ -86,7 +86,7 @@ pub const UNNAMED_TABLE: &str = "?table?"; /// # Ok(()) /// # } /// ``` -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct LogicalPlanBuilder { plan: LogicalPlan, } @@ -267,27 +267,27 @@ impl LogicalPlanBuilder { } /// Apply a projection without alias. pub fn project( - &self, + self, expr: impl IntoIterator>, ) -> Result { - Ok(Self::from(project(self.plan.clone(), expr)?)) + Ok(Self::from(project(self.plan, expr)?)) } /// Apply a filter - pub fn filter(&self, expr: impl Into) -> Result { + pub fn filter(self, expr: impl Into) -> Result { let expr = normalize_col(expr.into(), &self.plan)?; Ok(Self::from(LogicalPlan::Filter(Filter::try_new( expr, - Arc::new(self.plan.clone()), + Arc::new(self.plan), )?))) } /// Make a builder for a prepare logical plan from the builder's plan - pub fn prepare(&self, name: String, data_types: Vec) -> Result { + pub fn prepare(self, name: String, data_types: Vec) -> Result { Ok(Self::from(LogicalPlan::Prepare(Prepare { name, data_types, - input: Arc::new(self.plan.clone()), + input: Arc::new(self.plan), }))) } @@ -297,17 +297,17 @@ impl LogicalPlanBuilder { /// /// `fetch` - Maximum number of rows to fetch, after skipping `skip` rows, /// if specified. - pub fn limit(&self, skip: usize, fetch: Option) -> Result { + pub fn limit(self, skip: usize, fetch: Option) -> Result { Ok(Self::from(LogicalPlan::Limit(Limit { skip, fetch, - input: Arc::new(self.plan.clone()), + input: Arc::new(self.plan), }))) } /// Apply an alias - pub fn alias(&self, alias: impl Into) -> Result { - Ok(Self::from(subquery_alias(&self.plan, alias)?)) + pub fn alias(self, alias: impl Into) -> Result { + Ok(Self::from(subquery_alias(self.plan, alias)?)) } /// Add missing sort columns to all downstream projection @@ -353,7 +353,7 @@ impl LogicalPlanBuilder { /// Apply a sort pub fn sort( - &self, + self, exprs: impl IntoIterator> + Clone, ) -> Result { let exprs = rewrite_sort_cols_by_aggs(exprs, &self.plan)?; @@ -380,17 +380,11 @@ impl LogicalPlanBuilder { if missing_cols.is_empty() { return Ok(Self::from(LogicalPlan::Sort(Sort { expr: normalize_cols(exprs, &self.plan)?, - input: Arc::new(self.plan.clone()), + input: Arc::new(self.plan), fetch: None, }))); } - let plan = Self::add_missing_columns(self.plan.clone(), &missing_cols)?; - let sort_plan = LogicalPlan::Sort(Sort { - expr: normalize_cols(exprs, &plan)?, - input: Arc::new(plan.clone()), - fetch: None, - }); // remove pushed down sort columns let new_expr = schema .fields() @@ -398,6 +392,13 @@ impl LogicalPlanBuilder { .map(|f| Expr::Column(f.qualified_column())) .collect(); + let plan = Self::add_missing_columns(self.plan, &missing_cols)?; + let sort_plan = LogicalPlan::Sort(Sort { + expr: normalize_cols(exprs, &plan)?, + input: Arc::new(plan), + fetch: None, + }); + Ok(Self::from(LogicalPlan::Projection(Projection::try_new( new_expr, Arc::new(sort_plan), @@ -405,17 +406,16 @@ impl LogicalPlanBuilder { } /// Apply a union, preserving duplicate rows - pub fn union(&self, plan: LogicalPlan) -> Result { - Ok(Self::from(union(self.plan.clone(), plan)?)) + pub fn union(self, plan: LogicalPlan) -> Result { + Ok(Self::from(union(self.plan, plan)?)) } /// Apply a union, removing duplicate rows - pub fn union_distinct(&self, plan: LogicalPlan) -> Result { + pub fn union_distinct(self, plan: LogicalPlan) -> Result { // unwrap top-level Distincts, to avoid duplication - let left_plan = self.plan.clone(); - let left_plan: LogicalPlan = match left_plan { + let left_plan: LogicalPlan = match self.plan { LogicalPlan::Distinct(Distinct { input }) => (*input).clone(), - _ => left_plan, + _ => self.plan, }; let right_plan: LogicalPlan = match plan { LogicalPlan::Distinct(Distinct { input }) => (*input).clone(), @@ -428,9 +428,9 @@ impl LogicalPlanBuilder { } /// Apply deduplication: Only distinct (different) values are returned) - pub fn distinct(&self) -> Result { + pub fn distinct(self) -> Result { Ok(Self::from(LogicalPlan::Distinct(Distinct { - input: Arc::new(self.plan.clone()), + input: Arc::new(self.plan), }))) } @@ -440,8 +440,8 @@ impl LogicalPlanBuilder { /// down to any of join inputs. /// In case of outer join, filter applied to only matched rows. pub fn join( - &self, - right: &LogicalPlan, + self, + right: LogicalPlan, join_type: JoinType, join_keys: (Vec>, Vec>), filter: Option, @@ -463,8 +463,8 @@ impl LogicalPlanBuilder { /// Apply a join with on constraint and specified null equality /// If null_equals_null is true then null == null, else null != null pub fn join_detailed( - &self, - right: &LogicalPlan, + self, + right: LogicalPlan, join_type: JoinType, join_keys: (Vec>, Vec>), filter: Option, @@ -501,7 +501,7 @@ impl LogicalPlanBuilder { (Ok(_), _, _, Ok(_)) => (Ok(l), Ok(r)), _ => ( Self::normalize(&self.plan, l), - Self::normalize(right, r), + Self::normalize(&right, r), ), } } @@ -512,11 +512,11 @@ impl LogicalPlanBuilder { right.schema().field_with_qualified_name(lr, &l.name); match (l_is_left, l_is_right) { - (Ok(_), _) => (Ok(l), Self::normalize(right, r)), + (Ok(_), _) => (Ok(l), Self::normalize(&right, r)), (_, Ok(_)) => (Self::normalize(&self.plan, r), Ok(l)), _ => ( Self::normalize(&self.plan, l), - Self::normalize(right, r), + Self::normalize(&right, r), ), } } @@ -527,11 +527,11 @@ impl LogicalPlanBuilder { right.schema().field_with_qualified_name(rr, &r.name); match (r_is_left, r_is_right) { - (Ok(_), _) => (Ok(r), Self::normalize(right, l)), + (Ok(_), _) => (Ok(r), Self::normalize(&right, l)), (_, Ok(_)) => (Self::normalize(&self.plan, l), Ok(r)), _ => ( Self::normalize(&self.plan, l), - Self::normalize(right, r), + Self::normalize(&right, r), ), } } @@ -540,12 +540,12 @@ impl LogicalPlanBuilder { let left_key = Self::normalize(&self.plan, l.clone()) .or_else(|_| { swap = true; - Self::normalize(right, l) + Self::normalize(&right, l) }); if swap { (Self::normalize(&self.plan, r), left_key) } else { - (left_key, Self::normalize(right, r)) + (left_key, Self::normalize(&right, r)) } } } @@ -560,8 +560,8 @@ impl LogicalPlanBuilder { build_join_schema(self.plan.schema(), right.schema(), &join_type)?; Ok(Self::from(LogicalPlan::Join(Join { - left: Arc::new(self.plan.clone()), - right: Arc::new(right.clone()), + left: Arc::new(self.plan), + right: Arc::new(right), on, filter, join_type, @@ -573,8 +573,8 @@ impl LogicalPlanBuilder { /// Apply a join with using constraint, which duplicates all join columns in output schema. pub fn join_using( - &self, - right: &LogicalPlan, + self, + right: LogicalPlan, join_type: JoinType, using_keys: Vec + Clone>, ) -> Result { @@ -585,7 +585,7 @@ impl LogicalPlanBuilder { .collect::>()?; let right_keys: Vec = using_keys .into_iter() - .map(|c| Self::normalize(right, c)) + .map(|c| Self::normalize(&right, c)) .collect::>()?; let on: Vec<(_, _)> = left_keys.into_iter().zip(right_keys.into_iter()).collect(); @@ -617,14 +617,14 @@ impl LogicalPlanBuilder { } } if join_on.is_empty() { - let join = Self::from(self.plan.clone()).cross_join(&right.clone())?; + let join = Self::from(self.plan).cross_join(right)?; join.filter(filters.ok_or_else(|| { DataFusionError::Internal("filters should not be None here".to_string()) })?) } else { Ok(Self::from(LogicalPlan::Join(Join { - left: Arc::new(self.plan.clone()), - right: Arc::new(right.clone()), + left: Arc::new(self.plan), + right: Arc::new(right), on: join_on, filter: filters, join_type, @@ -636,26 +636,26 @@ impl LogicalPlanBuilder { } /// Apply a cross join - pub fn cross_join(&self, right: &LogicalPlan) -> Result { + pub fn cross_join(self, right: LogicalPlan) -> Result { let schema = self.plan.schema().join(right.schema())?; Ok(Self::from(LogicalPlan::CrossJoin(CrossJoin { - left: Arc::new(self.plan.clone()), - right: Arc::new(right.clone()), + left: Arc::new(self.plan), + right: Arc::new(right), schema: DFSchemaRef::new(schema), }))) } /// Repartition - pub fn repartition(&self, partitioning_scheme: Partitioning) -> Result { + pub fn repartition(self, partitioning_scheme: Partitioning) -> Result { Ok(Self::from(LogicalPlan::Repartition(Repartition { - input: Arc::new(self.plan.clone()), + input: Arc::new(self.plan), partitioning_scheme, }))) } /// Apply a window functions to extend the schema pub fn window( - &self, + self, window_expr: impl IntoIterator>, ) -> Result { let window_expr = normalize_cols(window_expr, &self.plan)?; @@ -663,13 +663,12 @@ impl LogicalPlanBuilder { validate_unique_names("Windows", all_expr.clone())?; let mut window_fields: Vec = exprlist_to_fields(all_expr, &self.plan)?; window_fields.extend_from_slice(self.plan.schema().fields()); + let metadata = self.plan.schema().metadata().clone(); + Ok(Self::from(LogicalPlan::Window(Window { - input: Arc::new(self.plan.clone()), + input: Arc::new(self.plan), window_expr, - schema: Arc::new(DFSchema::new_with_metadata( - window_fields, - self.plan.schema().metadata().clone(), - )?), + schema: Arc::new(DFSchema::new_with_metadata(window_fields, metadata)?), }))) } @@ -677,14 +676,14 @@ impl LogicalPlanBuilder { /// and calculating `aggr_expr` aggregates for each distinct /// value of the `group_expr`; pub fn aggregate( - &self, + self, group_expr: impl IntoIterator>, aggr_expr: impl IntoIterator>, ) -> Result { let group_expr = normalize_cols(group_expr, &self.plan)?; let aggr_expr = normalize_cols(aggr_expr, &self.plan)?; Ok(Self::from(LogicalPlan::Aggregate(Aggregate::try_new( - Arc::new(self.plan.clone()), + Arc::new(self.plan), group_expr, aggr_expr, )?))) @@ -696,14 +695,14 @@ impl LogicalPlanBuilder { /// information about metrics during run. /// /// if `verbose` is true, prints out additional details. - pub fn explain(&self, verbose: bool, analyze: bool) -> Result { + pub fn explain(self, verbose: bool, analyze: bool) -> Result { let schema = LogicalPlan::explain_schema(); let schema = schema.to_dfschema_ref()?; if analyze { Ok(Self::from(LogicalPlan::Analyze(Analyze { verbose, - input: Arc::new(self.plan.clone()), + input: Arc::new(self.plan), schema, }))) } else { @@ -712,7 +711,7 @@ impl LogicalPlanBuilder { Ok(Self::from(LogicalPlan::Explain(Explain { verbose, - plan: Arc::new(self.plan.clone()), + plan: Arc::new(self.plan), stringified_plans, schema, }))) @@ -778,19 +777,19 @@ impl LogicalPlanBuilder { .unzip(); if is_all { LogicalPlanBuilder::from(left_plan) - .join_detailed(&right_plan, join_type, join_keys, None, true)? + .join_detailed(right_plan, join_type, join_keys, None, true)? .build() } else { LogicalPlanBuilder::from(left_plan) .distinct()? - .join_detailed(&right_plan, join_type, join_keys, None, true)? + .join_detailed(right_plan, join_type, join_keys, None, true)? .build() } } /// Build the plan - pub fn build(&self) -> Result { - Ok(self.plan.clone()) + pub fn build(self) -> Result { + Ok(self.plan) } } @@ -978,12 +977,11 @@ pub fn project( /// Create a SubqueryAlias to wrap a LogicalPlan. pub fn subquery_alias( - plan: &LogicalPlan, + plan: LogicalPlan, alias: impl Into, ) -> Result { Ok(LogicalPlan::SubqueryAlias(SubqueryAlias::try_new( - plan.clone(), - alias, + plan, alias, )?)) } @@ -1177,7 +1175,7 @@ mod tests { let t2 = table_scan(Some("t2"), &employee_schema(), None)?.build()?; let plan = table_scan(Some("t1"), &employee_schema(), None)? - .join_using(&t2, JoinType::Inner, vec!["id"])? + .join_using(t2, JoinType::Inner, vec!["id"])? .project(vec![Expr::Wildcard])? .build()?; @@ -1198,8 +1196,9 @@ mod tests { table_scan(Some("employee_csv"), &employee_schema(), Some(vec![3, 4]))?; let plan = plan - .union(plan.build()?)? - .union(plan.build()?)? + .clone() + .union(plan.clone().build()?)? + .union(plan.clone().build()?)? .union(plan.build()?)? .build()?; @@ -1221,8 +1220,9 @@ mod tests { table_scan(Some("employee_csv"), &employee_schema(), Some(vec![3, 4]))?; let plan = plan - .union_distinct(plan.build()?)? - .union_distinct(plan.build()?)? + .clone() + .union_distinct(plan.clone().build()?)? + .union_distinct(plan.clone().build()?)? .union_distinct(plan.build()?)? .build()?; @@ -1247,7 +1247,7 @@ mod tests { let plan2 = table_scan(None, &employee_schema(), Some(vec![3, 4]))?; let expected = "Error during planning: Union queries must have the same number of columns, (left is 1, right is 2)"; - let err_msg1 = plan1.union(plan2.build()?).unwrap_err(); + let err_msg1 = plan1.clone().union(plan2.clone().build()?).unwrap_err(); let err_msg2 = plan1.union_distinct(plan2.build()?).unwrap_err(); assert_eq!(err_msg1.to_string(), expected); diff --git a/datafusion/expr/src/utils.rs b/datafusion/expr/src/utils.rs index e920915c088be..ec598ef413a02 100644 --- a/datafusion/expr/src/utils.rs +++ b/datafusion/expr/src/utils.rs @@ -497,7 +497,7 @@ pub fn from_plan( } LogicalPlan::CrossJoin(_) => { let left = inputs[0].clone(); - let right = &inputs[1]; + let right = inputs[1].clone(); LogicalPlanBuilder::from(left).cross_join(right)?.build() } LogicalPlan::Subquery(_) => { diff --git a/datafusion/optimizer/src/common_subexpr_eliminate.rs b/datafusion/optimizer/src/common_subexpr_eliminate.rs index b541aa0ee76ca..53fd28ee93bf2 100644 --- a/datafusion/optimizer/src/common_subexpr_eliminate.rs +++ b/datafusion/optimizer/src/common_subexpr_eliminate.rs @@ -797,7 +797,7 @@ mod test { let table_scan_1 = test_table_scan_with_name("test1").unwrap(); let table_scan_2 = test_table_scan_with_name("test2").unwrap(); let join = LogicalPlanBuilder::from(table_scan_1) - .join(&table_scan_2, JoinType::Inner, (vec!["a"], vec!["a"]), None) + .join(table_scan_2, JoinType::Inner, (vec!["a"], vec!["a"]), None) .unwrap() .build() .unwrap(); diff --git a/datafusion/optimizer/src/decorrelate_where_exists.rs b/datafusion/optimizer/src/decorrelate_where_exists.rs index 6b5bd184f5042..11c2d88b9081c 100644 --- a/datafusion/optimizer/src/decorrelate_where_exists.rs +++ b/datafusion/optimizer/src/decorrelate_where_exists.rs @@ -206,7 +206,7 @@ fn optimize_exists( false => JoinType::LeftSemi, }; let mut new_plan = LogicalPlanBuilder::from(outer_input.clone()).join( - &subqry_plan, + subqry_plan, join_type, join_keys, join_filters, diff --git a/datafusion/optimizer/src/decorrelate_where_in.rs b/datafusion/optimizer/src/decorrelate_where_in.rs index 4614b0699cc7a..ac4ca7c28f3f0 100644 --- a/datafusion/optimizer/src/decorrelate_where_in.rs +++ b/datafusion/optimizer/src/decorrelate_where_in.rs @@ -212,7 +212,7 @@ fn optimize_where_in( false => JoinType::LeftSemi, }; let mut new_plan = LogicalPlanBuilder::from(outer_input.clone()).join( - &subqry_plan, + subqry_plan, join_type, join_keys, join_filters, diff --git a/datafusion/optimizer/src/eliminate_cross_join.rs b/datafusion/optimizer/src/eliminate_cross_join.rs index b431742f0acf1..ae74e62efa4da 100644 --- a/datafusion/optimizer/src/eliminate_cross_join.rs +++ b/datafusion/optimizer/src/eliminate_cross_join.rs @@ -420,7 +420,7 @@ mod tests { // could eliminate to inner join since filter has Join predicates let plan = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? + .cross_join(t2)? .filter(binary_expr( col("t1.a").eq(col("t2.a")), And, @@ -448,7 +448,7 @@ mod tests { // could not eliminate to inner join since filter OR expression and there is no common // Join predicates in left and right of OR expr. let plan = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? + .cross_join(t2)? .filter(binary_expr( col("t1.a").eq(col("t2.a")), Or, @@ -475,7 +475,7 @@ mod tests { // could eliminate to inner join let plan = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? + .cross_join(t2)? .filter(binary_expr( binary_expr(col("t1.a").eq(col("t2.a")), And, col("t2.c").lt(lit(20u32))), And, @@ -502,7 +502,7 @@ mod tests { // could eliminate to inner join since Or predicates have common Join predicates let plan = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? + .cross_join(t2)? .filter(binary_expr( binary_expr(col("t1.a").eq(col("t2.a")), And, col("t2.c").lt(lit(15u32))), Or, @@ -532,7 +532,7 @@ mod tests { // could not eliminate to inner join let plan = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? + .cross_join(t2)? .filter(binary_expr( binary_expr(col("t1.a").eq(col("t2.a")), And, col("t2.c").lt(lit(15u32))), Or, @@ -562,7 +562,7 @@ mod tests { // could not eliminate to inner join let plan = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? + .cross_join(t2)? .filter(binary_expr( binary_expr(col("t1.a").eq(col("t2.a")), And, col("t2.c").lt(lit(15u32))), Or, @@ -604,8 +604,8 @@ mod tests { // could eliminate to inner join let plan = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? - .cross_join(&t3)? + .cross_join(t2)? + .cross_join(t3)? .filter(binary_expr( binary_expr(col("t3.a").eq(col("t1.a")), And, col("t3.c").lt(lit(15u32))), And, @@ -637,7 +637,7 @@ mod tests { // could eliminate to inner join let plan1 = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? + .cross_join(t2)? .filter(binary_expr( binary_expr(col("t1.a").eq(col("t2.a")), And, col("t2.c").lt(lit(15u32))), Or, @@ -650,7 +650,7 @@ mod tests { .build()?; let plan2 = LogicalPlanBuilder::from(t3) - .cross_join(&t4)? + .cross_join(t4)? .filter(binary_expr( binary_expr( binary_expr( @@ -675,7 +675,7 @@ mod tests { .build()?; let plan = LogicalPlanBuilder::from(plan1) - .cross_join(&plan2)? + .cross_join(plan2)? .filter(binary_expr( binary_expr(col("t3.a").eq(col("t1.a")), And, col("t4.c").lt(lit(15u32))), Or, @@ -714,7 +714,7 @@ mod tests { // could eliminate to inner join let plan1 = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? + .cross_join(t2)? .filter(binary_expr( binary_expr(col("t1.a").eq(col("t2.a")), And, col("t2.c").lt(lit(15u32))), Or, @@ -728,7 +728,7 @@ mod tests { // could eliminate to inner join let plan2 = LogicalPlanBuilder::from(t3) - .cross_join(&t4)? + .cross_join(t4)? .filter(binary_expr( binary_expr( binary_expr( @@ -754,7 +754,7 @@ mod tests { // could not eliminate to inner join let plan = LogicalPlanBuilder::from(plan1) - .cross_join(&plan2)? + .cross_join(plan2)? .filter(binary_expr( binary_expr(col("t3.a").eq(col("t1.a")), And, col("t4.c").lt(lit(15u32))), Or, @@ -789,7 +789,7 @@ mod tests { // could eliminate to inner join let plan1 = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? + .cross_join(t2)? .filter(binary_expr( binary_expr(col("t1.a").eq(col("t2.a")), And, col("t2.c").lt(lit(15u32))), Or, @@ -803,7 +803,7 @@ mod tests { // could not eliminate to inner join let plan2 = LogicalPlanBuilder::from(t3) - .cross_join(&t4)? + .cross_join(t4)? .filter(binary_expr( binary_expr( binary_expr( @@ -825,7 +825,7 @@ mod tests { // could eliminate to inner join let plan = LogicalPlanBuilder::from(plan1) - .cross_join(&plan2)? + .cross_join(plan2)? .filter(binary_expr( binary_expr(col("t3.a").eq(col("t1.a")), And, col("t4.c").lt(lit(15u32))), Or, @@ -864,7 +864,7 @@ mod tests { // could not eliminate to inner join let plan1 = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? + .cross_join(t2)? .filter(binary_expr( binary_expr(col("t1.a").eq(col("t2.a")), Or, col("t2.c").lt(lit(15u32))), Or, @@ -878,7 +878,7 @@ mod tests { // could eliminate to inner join let plan2 = LogicalPlanBuilder::from(t3) - .cross_join(&t4)? + .cross_join(t4)? .filter(binary_expr( binary_expr( binary_expr( @@ -904,7 +904,7 @@ mod tests { // could eliminate to inner join let plan = LogicalPlanBuilder::from(plan1) - .cross_join(&plan2)? + .cross_join(plan2)? .filter(binary_expr( binary_expr(col("t3.a").eq(col("t1.a")), And, col("t4.c").lt(lit(15u32))), Or, @@ -943,7 +943,7 @@ mod tests { // could eliminate to inner join let plan1 = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? + .cross_join(t2)? .filter(binary_expr( binary_expr(col("t1.a").eq(col("t2.a")), Or, col("t2.c").lt(lit(15u32))), And, @@ -956,11 +956,11 @@ mod tests { .build()?; // could eliminate to inner join - let plan2 = LogicalPlanBuilder::from(t3).cross_join(&t4)?.build()?; + let plan2 = LogicalPlanBuilder::from(t3).cross_join(t4)?.build()?; // could eliminate to inner join let plan = LogicalPlanBuilder::from(plan1) - .cross_join(&plan2)? + .cross_join(plan2)? .filter(binary_expr( binary_expr( binary_expr( @@ -1025,14 +1025,14 @@ mod tests { let t4 = test_table_scan_with_name("t4")?; // could eliminate to inner join - let plan1 = LogicalPlanBuilder::from(t1).cross_join(&t2)?.build()?; + let plan1 = LogicalPlanBuilder::from(t1).cross_join(t2)?.build()?; // could eliminate to inner join - let plan2 = LogicalPlanBuilder::from(t3).cross_join(&t4)?.build()?; + let plan2 = LogicalPlanBuilder::from(t3).cross_join(t4)?.build()?; // could eliminate to inner join let plan = LogicalPlanBuilder::from(plan1) - .cross_join(&plan2)? + .cross_join(plan2)? .filter(binary_expr( binary_expr( binary_expr( @@ -1111,7 +1111,7 @@ mod tests { // could eliminate to inner join since filter has Join predicates let plan = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? + .cross_join(t2)? .filter(binary_expr( (col("t1.a") + lit(100u32)).eq(col("t2.a") * lit(2u32)), And, @@ -1142,7 +1142,7 @@ mod tests { // could not eliminate to inner join since filter OR expression and there is no common // Join predicates in left and right of OR expr. let plan = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? + .cross_join(t2)? .filter(binary_expr( (col("t1.a") + lit(100u32)).eq(col("t2.a") * lit(2u32)), Or, @@ -1170,7 +1170,7 @@ mod tests { // could eliminate to inner join let common_join_key = (col("t1.a") + lit(100u32)).eq(col("t2.a") * lit(2u32)); let plan = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? + .cross_join(t2)? .filter(binary_expr( binary_expr(common_join_key.clone(), And, col("t2.c").lt(lit(20u32))), And, @@ -1201,7 +1201,7 @@ mod tests { // could eliminate to inner join since Or predicates have common Join predicates let common_join_key = (col("t1.a") + lit(100u32)).eq(col("t2.a") * lit(2u32)); let plan = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? + .cross_join(t2)? .filter(binary_expr( binary_expr(common_join_key.clone(), And, col("t2.c").lt(lit(15u32))), Or, @@ -1232,8 +1232,8 @@ mod tests { // could eliminate to inner join let plan = LogicalPlanBuilder::from(t1) - .cross_join(&t2)? - .cross_join(&t3)? + .cross_join(t2)? + .cross_join(t3)? .filter(binary_expr( binary_expr( (col("t3.a") + lit(100u32)).eq(col("t1.a") * lit(2u32)), diff --git a/datafusion/optimizer/src/eliminate_limit.rs b/datafusion/optimizer/src/eliminate_limit.rs index b60b16e83bc5a..b3e40a47522ba 100644 --- a/datafusion/optimizer/src/eliminate_limit.rs +++ b/datafusion/optimizer/src/eliminate_limit.rs @@ -233,7 +233,7 @@ mod tests { let plan = LogicalPlanBuilder::from(table_scan) .limit(2, Some(1))? .join_using( - &table_scan_inner, + table_scan_inner, JoinType::Inner, vec![Column::from_name("a".to_string())], )? diff --git a/datafusion/optimizer/src/eliminate_outer_join.rs b/datafusion/optimizer/src/eliminate_outer_join.rs index 4d1fecad70e89..1fcb348a557cf 100644 --- a/datafusion/optimizer/src/eliminate_outer_join.rs +++ b/datafusion/optimizer/src/eliminate_outer_join.rs @@ -346,7 +346,7 @@ mod tests { // could not eliminate to inner join let plan = LogicalPlanBuilder::from(t1) .join( - &t2, + t2, JoinType::Left, (vec![Column::from_name("a")], vec![Column::from_name("a")]), None, @@ -369,7 +369,7 @@ mod tests { // eliminate to inner join let plan = LogicalPlanBuilder::from(t1) .join( - &t2, + t2, JoinType::Left, (vec![Column::from_name("a")], vec![Column::from_name("a")]), None, @@ -392,7 +392,7 @@ mod tests { // eliminate to inner join let plan = LogicalPlanBuilder::from(t1) .join( - &t2, + t2, JoinType::Right, (vec![Column::from_name("a")], vec![Column::from_name("a")]), None, @@ -419,7 +419,7 @@ mod tests { // eliminate to inner join let plan = LogicalPlanBuilder::from(t1) .join( - &t2, + t2, JoinType::Full, (vec![Column::from_name("a")], vec![Column::from_name("a")]), None, @@ -446,7 +446,7 @@ mod tests { // eliminate to inner join let plan = LogicalPlanBuilder::from(t1) .join( - &t2, + t2, JoinType::Full, (vec![Column::from_name("a")], vec![Column::from_name("a")]), None, diff --git a/datafusion/optimizer/src/filter_null_join_keys.rs b/datafusion/optimizer/src/filter_null_join_keys.rs index f1a4f13937cd9..572c9451fb9ab 100644 --- a/datafusion/optimizer/src/filter_null_join_keys.rs +++ b/datafusion/optimizer/src/filter_null_join_keys.rs @@ -211,7 +211,7 @@ mod tests { let t3 = table_scan(Some("t3"), &schema, None)?.build()?; let plan = LogicalPlanBuilder::from(t3) .join( - &plan, + plan, JoinType::Inner, ( vec![ @@ -245,7 +245,7 @@ mod tests { ) -> Result { LogicalPlanBuilder::from(left_table) .join( - &right_table, + right_table, JoinType::Inner, ( vec![Column::from_qualified_name(left_key)], diff --git a/datafusion/optimizer/src/propagate_empty_relation.rs b/datafusion/optimizer/src/propagate_empty_relation.rs index cfdb354d4cb97..a3e85ef5409f4 100644 --- a/datafusion/optimizer/src/propagate_empty_relation.rs +++ b/datafusion/optimizer/src/propagate_empty_relation.rs @@ -272,7 +272,7 @@ mod tests { let plan = LogicalPlanBuilder::from(left) .join_using( - &right, + right, JoinType::Inner, vec![Column::from_name("a".to_string())], )? @@ -406,7 +406,7 @@ mod tests { let right = LogicalPlanBuilder::empty(false).build()?; let plan = LogicalPlanBuilder::from(left) - .cross_join(&right)? + .cross_join(right)? .filter(col("a").lt_eq(lit(1i64)))? .build()?; diff --git a/datafusion/optimizer/src/push_down_filter.rs b/datafusion/optimizer/src/push_down_filter.rs index 40ed00ebda04c..79481f82fb7cd 100644 --- a/datafusion/optimizer/src/push_down_filter.rs +++ b/datafusion/optimizer/src/push_down_filter.rs @@ -1143,6 +1143,7 @@ mod tests { .alias("test2")?; let plan = table + .clone() .union(table.build()?)? .filter(col("b").eq(lit(1i64)))? .build()?; @@ -1175,7 +1176,7 @@ mod tests { .build()?; let filter = and(col("test.a").eq(lit(1)), col("test1.d").gt(lit(2))); let plan = LogicalPlanBuilder::from(left) - .cross_join(&right)? + .cross_join(right)? .project(vec![col("test.a"), col("test1.d")])? .filter(filter)? .build()?; @@ -1204,7 +1205,7 @@ mod tests { .build()?; let filter = and(col("test.a").eq(lit(1)), col("test1.a").gt(lit(2))); let plan = LogicalPlanBuilder::from(left) - .cross_join(&right)? + .cross_join(right)? .project(vec![col("test.a"), col("test1.a")])? .filter(filter)? .build()?; @@ -1318,7 +1319,7 @@ mod tests { .build()?; let plan = LogicalPlanBuilder::from(left) .join( - &right, + right, JoinType::Inner, (vec![Column::from_name("a")], vec![Column::from_name("a")]), None, @@ -1358,7 +1359,7 @@ mod tests { .build()?; let plan = LogicalPlanBuilder::from(left) .join_using( - &right, + right, JoinType::Inner, vec![Column::from_name("a".to_string())], )? @@ -1399,7 +1400,7 @@ mod tests { .build()?; let plan = LogicalPlanBuilder::from(left) .join( - &right, + right, JoinType::Inner, (vec![Column::from_name("a")], vec![Column::from_name("a")]), None, @@ -1438,7 +1439,7 @@ mod tests { let plan = LogicalPlanBuilder::from(left) .join( - &right, + right, JoinType::Inner, (vec![Column::from_name("a")], vec![Column::from_name("a")]), None, @@ -1479,7 +1480,7 @@ mod tests { .build()?; let plan = LogicalPlanBuilder::from(left) .join_using( - &right, + right, JoinType::Left, vec![Column::from_name("a".to_string())], )? @@ -1517,7 +1518,7 @@ mod tests { .build()?; let plan = LogicalPlanBuilder::from(left) .join_using( - &right, + right, JoinType::Right, vec![Column::from_name("a".to_string())], )? @@ -1556,7 +1557,7 @@ mod tests { .build()?; let plan = LogicalPlanBuilder::from(left) .join_using( - &right, + right, JoinType::Left, vec![Column::from_name("a".to_string())], )? @@ -1595,7 +1596,7 @@ mod tests { .build()?; let plan = LogicalPlanBuilder::from(left) .join_using( - &right, + right, JoinType::Right, vec![Column::from_name("a".to_string())], )? @@ -1639,7 +1640,7 @@ mod tests { .and(col("test2.c").gt(lit(4u32))); let plan = LogicalPlanBuilder::from(left) .join( - &right, + right, JoinType::Inner, (vec![Column::from_name("a")], vec![Column::from_name("a")]), Some(filter), @@ -1683,7 +1684,7 @@ mod tests { .and(col("test2.c").gt(lit(4u32))); let plan = LogicalPlanBuilder::from(left) .join( - &right, + right, JoinType::Inner, (vec![Column::from_name("a")], vec![Column::from_name("a")]), Some(filter), @@ -1725,7 +1726,7 @@ mod tests { let filter = col("test.a").gt(lit(1u32)); let plan = LogicalPlanBuilder::from(left) .join( - &right, + right, JoinType::Inner, (vec![Column::from_name("a")], vec![Column::from_name("b")]), Some(filter), @@ -1770,7 +1771,7 @@ mod tests { .and(col("test2.c").gt(lit(4u32))); let plan = LogicalPlanBuilder::from(left) .join( - &right, + right, JoinType::Left, (vec![Column::from_name("a")], vec![Column::from_name("a")]), Some(filter), @@ -1814,7 +1815,7 @@ mod tests { .and(col("test2.c").gt(lit(4u32))); let plan = LogicalPlanBuilder::from(left) .join( - &right, + right, JoinType::Right, (vec![Column::from_name("a")], vec![Column::from_name("a")]), Some(filter), @@ -1858,7 +1859,7 @@ mod tests { .and(col("test2.c").gt(lit(4u32))); let plan = LogicalPlanBuilder::from(left) .join( - &right, + right, JoinType::Full, (vec![Column::from_name("a")], vec![Column::from_name("a")]), Some(filter), @@ -2110,7 +2111,7 @@ mod tests { let filter = col("c").gt(lit(1u32)); let plan = LogicalPlanBuilder::from(left) .join( - &right, + right, JoinType::Inner, (vec![Column::from_name("c")], vec![Column::from_name("d")]), Some(filter), @@ -2289,7 +2290,7 @@ mod tests { and(col("b").eq(col("e")), col("c").lt(lit(10u32))), ); let plan = LogicalPlanBuilder::from(left) - .cross_join(&right)? + .cross_join(right)? .filter(filter)? .build()?; diff --git a/datafusion/optimizer/src/push_down_limit.rs b/datafusion/optimizer/src/push_down_limit.rs index a404762b69ad8..54ff5807c26e2 100644 --- a/datafusion/optimizer/src/push_down_limit.rs +++ b/datafusion/optimizer/src/push_down_limit.rs @@ -545,7 +545,7 @@ mod test { let plan = LogicalPlanBuilder::from(table_scan_1) .join( - &LogicalPlanBuilder::from(table_scan_2).build()?, + LogicalPlanBuilder::from(table_scan_2).build()?, JoinType::Inner, (vec!["a"], vec!["a"]), None, @@ -569,7 +569,7 @@ mod test { let plan = LogicalPlanBuilder::from(table_scan_1) .join( - &LogicalPlanBuilder::from(table_scan_2).build()?, + LogicalPlanBuilder::from(table_scan_2).build()?, JoinType::Inner, (vec!["a"], vec!["a"]), None, @@ -652,7 +652,7 @@ mod test { let right_keys: Vec<&str> = Vec::new(); let plan = LogicalPlanBuilder::from(table_scan_1.clone()) .join( - &LogicalPlanBuilder::from(table_scan_2.clone()).build()?, + LogicalPlanBuilder::from(table_scan_2.clone()).build()?, JoinType::Left, (left_keys.clone(), right_keys.clone()), None, @@ -671,7 +671,7 @@ mod test { let plan = LogicalPlanBuilder::from(table_scan_1.clone()) .join( - &LogicalPlanBuilder::from(table_scan_2.clone()).build()?, + LogicalPlanBuilder::from(table_scan_2.clone()).build()?, JoinType::Right, (left_keys.clone(), right_keys.clone()), None, @@ -690,7 +690,7 @@ mod test { let plan = LogicalPlanBuilder::from(table_scan_1.clone()) .join( - &LogicalPlanBuilder::from(table_scan_2.clone()).build()?, + LogicalPlanBuilder::from(table_scan_2.clone()).build()?, JoinType::Full, (left_keys.clone(), right_keys.clone()), None, @@ -709,7 +709,7 @@ mod test { let plan = LogicalPlanBuilder::from(table_scan_1.clone()) .join( - &LogicalPlanBuilder::from(table_scan_2.clone()).build()?, + LogicalPlanBuilder::from(table_scan_2.clone()).build()?, JoinType::LeftSemi, (left_keys.clone(), right_keys.clone()), None, @@ -727,7 +727,7 @@ mod test { let plan = LogicalPlanBuilder::from(table_scan_1.clone()) .join( - &LogicalPlanBuilder::from(table_scan_2.clone()).build()?, + LogicalPlanBuilder::from(table_scan_2.clone()).build()?, JoinType::LeftAnti, (left_keys.clone(), right_keys.clone()), None, @@ -745,7 +745,7 @@ mod test { let plan = LogicalPlanBuilder::from(table_scan_1.clone()) .join( - &LogicalPlanBuilder::from(table_scan_2.clone()).build()?, + LogicalPlanBuilder::from(table_scan_2.clone()).build()?, JoinType::RightSemi, (left_keys.clone(), right_keys.clone()), None, @@ -763,7 +763,7 @@ mod test { let plan = LogicalPlanBuilder::from(table_scan_1) .join( - &LogicalPlanBuilder::from(table_scan_2).build()?, + LogicalPlanBuilder::from(table_scan_2).build()?, JoinType::RightAnti, (left_keys, right_keys), None, @@ -787,7 +787,7 @@ mod test { let plan = LogicalPlanBuilder::from(table_scan_1) .join( - &LogicalPlanBuilder::from(table_scan_2).build()?, + LogicalPlanBuilder::from(table_scan_2).build()?, JoinType::Left, (vec!["a"], vec!["a"]), None, @@ -812,7 +812,7 @@ mod test { let plan = LogicalPlanBuilder::from(table_scan_1) .join( - &LogicalPlanBuilder::from(table_scan_2).build()?, + LogicalPlanBuilder::from(table_scan_2).build()?, JoinType::Left, (vec!["a"], vec!["a"]), None, @@ -837,7 +837,7 @@ mod test { let plan = LogicalPlanBuilder::from(table_scan_1) .join( - &LogicalPlanBuilder::from(table_scan_2).build()?, + LogicalPlanBuilder::from(table_scan_2).build()?, JoinType::Right, (vec!["a"], vec!["a"]), None, @@ -862,7 +862,7 @@ mod test { let plan = LogicalPlanBuilder::from(table_scan_1) .join( - &LogicalPlanBuilder::from(table_scan_2).build()?, + LogicalPlanBuilder::from(table_scan_2).build()?, JoinType::Right, (vec!["a"], vec!["a"]), None, @@ -886,7 +886,7 @@ mod test { let table_scan_2 = test_table_scan_with_name("test2")?; let plan = LogicalPlanBuilder::from(table_scan_1) - .cross_join(&LogicalPlanBuilder::from(table_scan_2).build()?)? + .cross_join(LogicalPlanBuilder::from(table_scan_2).build()?)? .limit(0, Some(1000))? .build()?; @@ -906,7 +906,7 @@ mod test { let table_scan_2 = test_table_scan_with_name("test2")?; let plan = LogicalPlanBuilder::from(table_scan_1) - .cross_join(&LogicalPlanBuilder::from(table_scan_2).build()?)? + .cross_join(LogicalPlanBuilder::from(table_scan_2).build()?)? .limit(1000, Some(1000))? .build()?; diff --git a/datafusion/optimizer/src/push_down_projection.rs b/datafusion/optimizer/src/push_down_projection.rs index 0238148c3ec15..ff8c973d2dc27 100644 --- a/datafusion/optimizer/src/push_down_projection.rs +++ b/datafusion/optimizer/src/push_down_projection.rs @@ -676,7 +676,7 @@ mod tests { let table2_scan = scan_empty(Some("test2"), &schema, None)?.build()?; let plan = LogicalPlanBuilder::from(table_scan) - .join(&table2_scan, JoinType::Left, (vec!["a"], vec!["c1"]), None)? + .join(table2_scan, JoinType::Left, (vec!["a"], vec!["c1"]), None)? .project(vec![col("a"), col("b"), col("c1")])? .build()?; @@ -717,7 +717,7 @@ mod tests { let table2_scan = scan_empty(Some("test2"), &schema, None)?.build()?; let plan = LogicalPlanBuilder::from(table_scan) - .join(&table2_scan, JoinType::Left, (vec!["a"], vec!["c1"]), None)? + .join(table2_scan, JoinType::Left, (vec!["a"], vec!["c1"]), None)? // projecting joined column `a` should push the right side column `c1` projection as // well into test2 table even though `c1` is not referenced in projection. .project(vec![col("a"), col("b")])? @@ -760,7 +760,7 @@ mod tests { let table2_scan = scan_empty(Some("test2"), &schema, None)?.build()?; let plan = LogicalPlanBuilder::from(table_scan) - .join_using(&table2_scan, JoinType::Left, vec!["a"])? + .join_using(table2_scan, JoinType::Left, vec!["a"])? .project(vec![col("a"), col("b")])? .build()?; diff --git a/datafusion/optimizer/src/scalar_subquery_to_join.rs b/datafusion/optimizer/src/scalar_subquery_to_join.rs index 00e4d89c8df4b..89f927d6410a8 100644 --- a/datafusion/optimizer/src/scalar_subquery_to_join.rs +++ b/datafusion/optimizer/src/scalar_subquery_to_join.rs @@ -321,10 +321,10 @@ fn optimize_scalar( let new_plan = LogicalPlanBuilder::from(filter_input.clone()); let mut new_plan = if join_keys.0.is_empty() { // if not correlated, group down to 1 row and cross join on that (preserving row count) - new_plan.cross_join(&subqry_plan)? + new_plan.cross_join(subqry_plan)? } else { // inner join if correlated, grouping by the join keys so we don't change row count - new_plan.join(&subqry_plan, JoinType::Inner, join_keys, None)? + new_plan.join(subqry_plan, JoinType::Inner, join_keys, None)? }; // restore where in condition diff --git a/datafusion/proto/src/logical_plan.rs b/datafusion/proto/src/logical_plan.rs index d9423f8de69eb..09ae9e41e7ec0 100644 --- a/datafusion/proto/src/logical_plan.rs +++ b/datafusion/proto/src/logical_plan.rs @@ -728,13 +728,13 @@ impl AsLogicalPlan for LogicalPlanNode { )?); let builder = match join_constraint.into() { JoinConstraint::On => builder.join( - &into_logical_plan!(join.right, ctx, extension_codec)?, + into_logical_plan!(join.right, ctx, extension_codec)?, join_type.into(), (left_keys, right_keys), filter, )?, JoinConstraint::Using => builder.join_using( - &into_logical_plan!(join.right, ctx, extension_codec)?, + into_logical_plan!(join.right, ctx, extension_codec)?, join_type.into(), left_keys, )?, @@ -768,7 +768,7 @@ impl AsLogicalPlan for LogicalPlanNode { let left = into_logical_plan!(crossjoin.left, ctx, extension_codec)?; let right = into_logical_plan!(crossjoin.right, ctx, extension_codec)?; - LogicalPlanBuilder::from(left).cross_join(&right)?.build() + LogicalPlanBuilder::from(left).cross_join(right)?.build() } LogicalPlanType::Extension(LogicalExtensionNode { node, inputs }) => { let input_plans: Vec = inputs diff --git a/datafusion/sql/src/planner.rs b/datafusion/sql/src/planner.rs index 6cf279eda4178..4c73a4262e6e7 100644 --- a/datafusion/sql/src/planner.rs +++ b/datafusion/sql/src/planner.rs @@ -709,15 +709,16 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { self.plan_table_with_joins(from, planner_context) } _ => { - let plans = from + let mut plans = from .into_iter() - .map(|t| self.plan_table_with_joins(t, planner_context)) - .collect::>>()?; - let mut left = plans[0].clone(); - for right in plans.iter().skip(1) { - left = LogicalPlanBuilder::from(left).cross_join(right)?.build()?; + .map(|t| self.plan_table_with_joins(t, planner_context)); + + let mut left = LogicalPlanBuilder::from(plans.next().unwrap()?); + + for right in plans { + left = left.cross_join(right?)?; } - Ok(left) + Ok(left.build()?) } } } @@ -777,7 +778,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { JoinOperator::FullOuter(constraint) => { self.parse_join(left, right, constraint, JoinType::Full, planner_context) } - JoinOperator::CrossJoin => self.parse_cross_join(left, &right), + JoinOperator::CrossJoin => self.parse_cross_join(left, right), other => Err(DataFusionError::NotImplemented(format!( "Unsupported JOIN operator {:?}", other @@ -788,7 +789,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { fn parse_cross_join( &self, left: LogicalPlan, - right: &LogicalPlan, + right: LogicalPlan, ) -> Result { LogicalPlanBuilder::from(left).cross_join(right)?.build() } @@ -843,11 +844,11 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { if left_keys.is_empty() { // TODO should not use cross join when the join_filter exists // https://github.com/apache/arrow-datafusion/issues/4363 - let join = LogicalPlanBuilder::from(left).cross_join(&right)?; - join_filter - .map(|filter| join.filter(filter)) - .unwrap_or(Ok(join))? - .build() + let mut join = LogicalPlanBuilder::from(left).cross_join(right)?; + if let Some(filter) = join_filter { + join = join.filter(filter)?; + } + join.build() } else { // Wrap projection for left input if left join keys contain normal expression. let (left_child, left_join_keys, left_projected) = @@ -858,7 +859,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { wrap_projection_for_join_if_necessary(&right_keys, right)?; let join_plan_builder = LogicalPlanBuilder::from(left_child).join( - &right_child, + right_child, join_type, (left_join_keys, right_join_keys), join_filter, @@ -883,7 +884,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { .map(|x| Column::from_name(normalize_ident(x))) .collect(); LogicalPlanBuilder::from(left) - .join_using(&right, join_type, keys)? + .join_using(right, join_type, keys)? .build() } JoinConstraint::Natural => {