Skip to content

Commit

Permalink
ARROW-8243: [Rust] [DataFusion] Fix inconsistency in LogicalPlanBuild…
Browse files Browse the repository at this point in the history
…er api

LogicalPlanBuilder project method takes a &Vec whereas other methods take a Vec. It makes sense to take Vec and take ownership of these inputs since they are being used to build the plan.

Closes #6741 from andygrove/ARROW-8243

Authored-by: Andy Grove <andygrove73@gmail.com>
Signed-off-by: Andy Grove <andygrove73@gmail.com>
  • Loading branch information
andygrove committed Mar 27, 2020
1 parent 8ee4be7 commit 6ec0d77
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 8 deletions.
2 changes: 1 addition & 1 deletion rust/datafusion/src/execution/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,7 @@ mod tests {
vec![col(0)],
vec![aggregate_expr("SUM", col(1), DataType::Int32)],
)?
.project(&vec![col(0), col(1).alias("total_salary")])?
.project(vec![col(0), col(1).alias("total_salary")])?
.build()?;

let physical_plan = ctx.create_physical_plan(&Arc::new(plan), 1024)?;
Expand Down
8 changes: 4 additions & 4 deletions rust/datafusion/src/logicalplan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ impl LogicalPlanBuilder {
}

/// Apply a projection
pub fn project(&self, expr: &Vec<Expr>) -> Result<Self> {
pub fn project(&self, expr: Vec<Expr>) -> Result<Self> {
let input_schema = self.plan.schema();
let projected_expr = if expr.contains(&Expr::Wildcard) {
let mut expr_vec = vec![];
Expand Down Expand Up @@ -792,7 +792,7 @@ mod tests {
Some(vec![0, 3]),
)?
.filter(col(1).eq(&lit_str("CO")))?
.project(&vec![col(0)])?
.project(vec![col(0)])?
.build()?;

// prove that a plan can be passed to a thread
Expand All @@ -813,7 +813,7 @@ mod tests {
Some(vec![0, 3]),
)?
.filter(col(1).eq(&lit_str("CO")))?
.project(&vec![col(0)])?
.project(vec![col(0)])?
.build()?;

let expected = "Projection: #0\n \
Expand All @@ -837,7 +837,7 @@ mod tests {
vec![col(0)],
vec![aggregate_expr("SUM", col(1), DataType::Int32)],
)?
.project(&vec![col(0), col(1).alias("total_salary")])?
.project(vec![col(0), col(1).alias("total_salary")])?
.build()?;

let expected = "Projection: #0, #1 AS total_salary\
Expand Down
6 changes: 3 additions & 3 deletions rust/datafusion/src/sql/planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ impl<S: SchemaProvider> SqlToRel<S> {
let plan = if group_by.is_some() || aggr_expr.len() > 0 {
self.aggregate(&plan, projection_expr, group_by, aggr_expr)?
} else {
self.project(&plan, &projection_expr)?
self.project(&plan, projection_expr)?
};

// apply ORDER BY
Expand Down Expand Up @@ -140,7 +140,7 @@ impl<S: SchemaProvider> SqlToRel<S> {
}

/// Wrap a plan in a projection
fn project(&self, input: &LogicalPlan, expr: &Vec<Expr>) -> Result<LogicalPlan> {
fn project(&self, input: &LogicalPlan, expr: Vec<Expr>) -> Result<LogicalPlan> {
LogicalPlanBuilder::from(input).project(expr)?.build()
}

Expand Down Expand Up @@ -200,7 +200,7 @@ impl<S: SchemaProvider> SqlToRel<S> {
if projection_needed {
self.project(
&plan,
&projected_fields.iter().map(|i| Expr::Column(*i)).collect(),
projected_fields.iter().map(|i| Expr::Column(*i)).collect(),
)
} else {
Ok(plan)
Expand Down

0 comments on commit 6ec0d77

Please sign in to comment.