Skip to content

Commit

Permalink
fix window schema handling in projection pushdown optimizer
Browse files Browse the repository at this point in the history
  • Loading branch information
houqp committed Jun 20, 2021
1 parent 071d8ac commit d26b54c
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 27 deletions.
32 changes: 11 additions & 21 deletions datafusion/src/optimizer/projection_push_down.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
use crate::error::Result;
use crate::execution::context::ExecutionProps;
use crate::logical_plan::{
build_join_schema, Column, DFField, DFSchema, DFSchemaRef, LogicalPlan, ToDFSchema,
build_join_schema, Column, DFField, DFSchema, DFSchemaRef, LogicalPlan,
LogicalPlanBuilder, ToDFSchema,
};
use crate::optimizer::optimizer::OptimizerRule;
use crate::optimizer::utils;
Expand Down Expand Up @@ -253,26 +254,15 @@ fn optimize_plan(
&mut new_required_columns,
)?;

let new_schema = DFSchema::new(
schema
.fields()
.iter()
.filter(|f| new_required_columns.contains(&f.qualified_column()))
.cloned()
.collect(),
)?;

Ok(LogicalPlan::Window {
window_expr: new_window_expr,
input: Arc::new(optimize_plan(
optimizer,
input,
&new_required_columns,
true,
execution_props,
)?),
schema: DFSchemaRef::new(new_schema),
})
LogicalPlanBuilder::from(&optimize_plan(

This comment has been minimized.

Copy link
@alamb

alamb Jun 21, 2021

Contributor

👍

optimizer,
input,
&new_required_columns,
true,
execution_props,
)?)
.window(new_window_expr)?
.build()
}
LogicalPlan::Aggregate {
schema,
Expand Down
6 changes: 2 additions & 4 deletions datafusion/src/physical_plan/planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,10 +265,8 @@ impl DefaultPhysicalPlanner {
}

let input_exec = self.create_initial_plan(input, ctx_state)?;
let input_schema = input_exec.schema();

let physical_input_schema = input_exec.schema();
let logical_input_schema = input.as_ref().schema();
let physical_input_schema = input_exec.as_ref().schema();

let window_expr = window_expr
.iter()
Expand All @@ -285,7 +283,7 @@ impl DefaultPhysicalPlanner {
Ok(Arc::new(WindowAggExec::try_new(
window_expr,
input_exec.clone(),
input_schema,
physical_input_schema,
)?))
}
LogicalPlan::Aggregate {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/src/physical_plan/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ impl WindowAggExec {
input: Arc<dyn ExecutionPlan>,
input_schema: SchemaRef,
) -> Result<Self> {
let schema = create_schema(&input.schema(), &window_expr)?;
let schema = create_schema(&input_schema, &window_expr)?;
let schema = Arc::new(schema);
Ok(WindowAggExec {
input,
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/test_psql_parity.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def test_parity(self):
psql_output = pd.read_csv(io.BytesIO(generate_csv_from_psql(fname)))
self.assertTrue(
np.allclose(datafusion_output, psql_output),
msg=f"data fusion output={datafusion_output}, psql_output={psql_output}",
msg=f"datafusion output=\n{datafusion_output}, psql_output=\n{psql_output}",
)


Expand Down

0 comments on commit d26b54c

Please sign in to comment.