Skip to content

fix: display the content of debug explain #434

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 1, 2021
Merged

fix: display the content of debug explain #434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
124ec3f
test: display of each plan
May 27, 2021
1926591
fix: Fix debug display of explain and potential fix of predicate push…
May 28, 2021
9dbd9f1
refactor: adddress Andrew's comments
Jun 1, 2021
373a347
fix: avoid comparing file path that are test environmentally dependant
Jun 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions datafusion/src/logical_plan/plan.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,11 +329,11 @@ impl LogicalPlan {
LogicalPlan::Limit { input, .. } => vec![input],
LogicalPlan::Extension { node } => node.inputs(),
LogicalPlan::Union { inputs, .. } => inputs.iter().collect(),
LogicalPlan::Explain { plan, .. } => vec![plan],
Copy link
Contributor Author

@NGA-TRAN NGA-TRAN May 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the fix that progpagates the optimization down the plan explain

// plans without inputs
LogicalPlan::TableScan { .. }
| LogicalPlan::EmptyRelation { .. }
| LogicalPlan::CreateExternalTable { .. }
| LogicalPlan::Explain { .. } => vec![],
| LogicalPlan::CreateExternalTable { .. } => vec![],
}
}
}
Expand Down Expand Up @@ -438,11 +438,11 @@ impl LogicalPlan {
}
true
}
LogicalPlan::Explain { plan, .. } => plan.accept(visitor)?,
Copy link
Contributor Author

@NGA-TRAN NGA-TRAN May 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the fix that enables LogicalPlan's display_... functions to show all info of the plan. The newly added tests capture this fix.

// plans without inputs
LogicalPlan::TableScan { .. }
| LogicalPlan::EmptyRelation { .. }
| LogicalPlan::CreateExternalTable { .. }
| LogicalPlan::Explain { .. } => true,
| LogicalPlan::CreateExternalTable { .. } => true,
};
if !recurse {
return Ok(false);
Expand Down
4 changes: 4 additions & 0 deletions datafusion/src/optimizer/filter_push_down.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,10 @@ fn split_members<'a>(predicate: &'a Expr, predicates: &mut Vec<&'a Expr>) {

fn optimize(plan: &LogicalPlan, mut state: State) -> Result<LogicalPlan> {
match plan {
LogicalPlan::Explain { .. } => {
// push the optimization to the plan of this explain
push_down(&state, plan)
}
Copy link
Contributor Author

@NGA-TRAN NGA-TRAN May 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is related to the fix to propagate the optimization down

LogicalPlan::Filter { input, predicate } => {
let mut predicates = vec![];
split_members(predicate, &mut predicates);
Expand Down
Loading