Skip to content

Commit a83ec4e

Browse files
alambalamb
committed
Remove to_stringified and simplify code
1 parent 114bf4b commit a83ec4e

File tree

3 files changed

+52
-93
lines changed

3 files changed

+52
-93
lines changed

‎datafusion-examples/examples/planner_api.rs‎

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
// under the License.
1717

1818
use datafusion::error::Result;
19-
use datafusion::logical_expr::{LogicalPlan, PlanType};
20-
use datafusion::physical_plan::{displayable, DisplayFormatType};
19+
use datafusion::logical_expr::LogicalPlan;
20+
use datafusion::physical_plan::displayable;
2121
use datafusion::physical_planner::DefaultPhysicalPlanner;
2222
use datafusion::prelude::*;
2323

@@ -77,13 +77,7 @@ async fn to_physical_plan_in_one_api_demo(
7777

7878
println!(
7979
"Physical plan direct from logical plan:\n\n{}\n\n",
80-
displayable(physical_plan.as_ref())
81-
.to_stringified(
82-
false,
83-
PlanType::InitialPhysicalPlan,
84-
DisplayFormatType::Default
85-
)
86-
.plan
80+
displayable(physical_plan.as_ref()).indent(false)
8781
);
8882

8983
Ok(())
@@ -123,13 +117,7 @@ async fn to_physical_plan_step_by_step_demo(
123117
.await?;
124118
println!(
125119
"Final physical plan:\n\n{}\n\n",
126-
displayable(physical_plan.as_ref())
127-
.to_stringified(
128-
false,
129-
PlanType::InitialPhysicalPlan,
130-
DisplayFormatType::Default
131-
)
132-
.plan
120+
displayable(physical_plan.as_ref()).indent(false)
133121
);
134122

135123
// Call the physical optimizer with an existing physical plan (in this
@@ -142,13 +130,7 @@ async fn to_physical_plan_step_by_step_demo(
142130
planner.optimize_physical_plan(physical_plan, &ctx.state(), |_, _| {})?;
143131
println!(
144132
"Optimized physical plan:\n\n{}\n\n",
145-
displayable(physical_plan.as_ref())
146-
.to_stringified(
147-
false,
148-
PlanType::InitialPhysicalPlan,
149-
DisplayFormatType::Default
150-
)
151-
.plan
133+
displayable(physical_plan.as_ref()).indent(false)
152134
);
153135

154136
Ok(())

‎datafusion/core/src/physical_planner.rs‎

Lines changed: 43 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ use datafusion_physical_optimizer::PhysicalOptimizerRule;
8888
use datafusion_physical_plan::execution_plan::InvariantLevel;
8989
use datafusion_physical_plan::placeholder_row::PlaceholderRowExec;
9090
use datafusion_physical_plan::unnest::ListUnnest;
91-
use datafusion_physical_plan::DisplayFormatType;
9291

9392
use crate::schema_equivalence::schema_satisfied_by;
9493
use async_trait::async_trait;
@@ -1755,13 +1754,12 @@ impl DefaultPhysicalPlanner {
17551754
|_plan, _optimizer| {},
17561755
)?;
17571756

1758-
stringified_plans.push(
1759-
displayable(optimized_plan.as_ref()).to_stringified(
1760-
e.verbose,
1761-
FinalPhysicalPlan,
1762-
DisplayFormatType::TreeRender,
1763-
),
1764-
);
1757+
stringified_plans.push(StringifiedPlan::new(
1758+
FinalPhysicalPlan,
1759+
displayable(optimized_plan.as_ref())
1760+
.tree_render()
1761+
.to_string(),
1762+
));
17651763
}
17661764
ExplainFormat::PostgresJSON => {
17671765
stringified_plans.push(StringifiedPlan::new(
@@ -1794,49 +1792,42 @@ impl DefaultPhysicalPlanner {
17941792
}
17951793
}
17961794

1797-
let display_format = DisplayFormatType::Default;
17981795
if !config.logical_plan_only && e.logical_optimization_succeeded {
17991796
match self
18001797
.create_initial_plan(e.plan.as_ref(), session_state)
18011798
.await
18021799
{
18031800
Ok(input) => {
18041801
// Include statistics / schema if enabled
1805-
stringified_plans.push(
1802+
stringified_plans.push(StringifiedPlan::new(
1803+
InitialPhysicalPlan,
18061804
displayable(input.as_ref())
18071805
.set_show_statistics(config.show_statistics)
18081806
.set_show_schema(config.show_schema)
1809-
.to_stringified(
1810-
e.verbose,
1811-
InitialPhysicalPlan,
1812-
display_format,
1813-
),
1814-
);
1807+
.indent(e.verbose)
1808+
.to_string(),
1809+
));
18151810

18161811
// Show statistics + schema in verbose output even if not
18171812
// explicitly requested
18181813
if e.verbose {
18191814
if !config.show_statistics {
1820-
stringified_plans.push(
1815+
stringified_plans.push(StringifiedPlan::new(
1816+
InitialPhysicalPlanWithStats,
18211817
displayable(input.as_ref())
18221818
.set_show_statistics(true)
1823-
.to_stringified(
1824-
e.verbose,
1825-
InitialPhysicalPlanWithStats,
1826-
display_format,
1827-
),
1828-
);
1819+
.indent(e.verbose)
1820+
.to_string(),
1821+
));
18291822
}
18301823
if !config.show_schema {
1831-
stringified_plans.push(
1824+
stringified_plans.push(StringifiedPlan::new(
1825+
InitialPhysicalPlanWithSchema,
18321826
displayable(input.as_ref())
18331827
.set_show_schema(true)
1834-
.to_stringified(
1835-
e.verbose,
1836-
InitialPhysicalPlanWithSchema,
1837-
display_format,
1838-
),
1839-
);
1828+
.indent(e.verbose)
1829+
.to_string(),
1830+
));
18401831
}
18411832
}
18421833

@@ -1846,52 +1837,50 @@ impl DefaultPhysicalPlanner {
18461837
|plan, optimizer| {
18471838
let optimizer_name = optimizer.name().to_string();
18481839
let plan_type = OptimizedPhysicalPlan { optimizer_name };
1849-
stringified_plans.push(
1840+
stringified_plans.push(StringifiedPlan::new(
1841+
plan_type,
18501842
displayable(plan)
18511843
.set_show_statistics(config.show_statistics)
18521844
.set_show_schema(config.show_schema)
1853-
.to_stringified(e.verbose, plan_type, display_format),
1854-
);
1845+
.indent(e.verbose)
1846+
.to_string(),
1847+
));
18551848
},
18561849
);
18571850
match optimized_plan {
18581851
Ok(input) => {
18591852
// This plan will includes statistics if show_statistics is on
1860-
stringified_plans.push(
1853+
stringified_plans.push(StringifiedPlan::new(
1854+
FinalPhysicalPlan,
18611855
displayable(input.as_ref())
18621856
.set_show_statistics(config.show_statistics)
18631857
.set_show_schema(config.show_schema)
1864-
.to_stringified(
1865-
e.verbose,
1866-
FinalPhysicalPlan,
1867-
display_format,
1868-
),
1869-
);
1858+
.indent(e.verbose)
1859+
.to_string(),
1860+
));
18701861

18711862
// Show statistics + schema in verbose output even if not
18721863
// explicitly requested
18731864
if e.verbose {
18741865
if !config.show_statistics {
1875-
stringified_plans.push(
1866+
stringified_plans.push(StringifiedPlan::new(
1867+
FinalPhysicalPlanWithStats,
18761868
displayable(input.as_ref())
18771869
.set_show_statistics(true)
1878-
.to_stringified(
1879-
e.verbose,
1880-
FinalPhysicalPlanWithStats,
1881-
display_format,
1882-
),
1883-
);
1870+
.indent(e.verbose)
1871+
.to_string(),
1872+
));
18841873
}
18851874
if !config.show_schema {
1886-
stringified_plans.push(
1875+
stringified_plans.push(StringifiedPlan::new(
1876+
FinalPhysicalPlanWithSchema,
1877+
// This will include schema if show_schema is on
1878+
// and will be set to true if verbose is on
18871879
displayable(input.as_ref())
18881880
.set_show_schema(true)
1889-
.to_stringified(
1890-
e.verbose,
1891-
FinalPhysicalPlanWithSchema,
1892-
display_format,
1893-
),
1894-
);
1881+
.indent(e.verbose)
1882+
.to_string(),
1883+
));
18951884
}
18961885
}
18971886
}

‎datafusion/physical-plan/src/display.rs‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use std::fmt::Formatter;
2424

2525
use arrow::datatypes::SchemaRef;
2626

27-
use datafusion_common::display::{GraphvizBuilder, PlanType, StringifiedPlan};
27+
use datafusion_common::display::GraphvizBuilder;
2828
use datafusion_expr::display_schema;
2929
use datafusion_physical_expr::LexOrdering;
3030

@@ -264,6 +264,9 @@ impl<'a> DisplayableExecutionPlan<'a> {
264264
}
265265
}
266266

267+
/// Formats the plan using a ASCII art like tree
268+
///
269+
/// See [`DisplayFormatType::TreeRender`] for more details.
267270
pub fn tree_render(&self) -> impl fmt::Display + 'a {
268271
struct Wrapper<'a> {
269272
plan: &'a dyn ExecutionPlan,
@@ -309,21 +312,6 @@ impl<'a> DisplayableExecutionPlan<'a> {
309312
show_schema: self.show_schema,
310313
}
311314
}
312-
313-
/// format as a `StringifiedPlan`
314-
pub fn to_stringified(
315-
&self,
316-
verbose: bool,
317-
plan_type: PlanType,
318-
explain_format: DisplayFormatType,
319-
) -> StringifiedPlan {
320-
match (&explain_format, &plan_type) {
321-
(DisplayFormatType::TreeRender, PlanType::FinalPhysicalPlan) => {
322-
StringifiedPlan::new(plan_type, self.tree_render().to_string())
323-
}
324-
_ => StringifiedPlan::new(plan_type, self.indent(verbose).to_string()),
325-
}
326-
}
327315
}
328316

329317
/// Enum representing the different levels of metrics to display

0 commit comments

Comments
 (0)