ARROW-10531: [Rust][DataFusion]: Add schema and graphviz formatting for LogicalPlans and a PlanVisitor #8619

alamb · 2020-11-09T22:57:45Z

Rationale:

I have been tracking down potential issues DataFusion for my work project, and I have found myself wanting to print out the state of the logical_plan several times. The existing debug formatting is ok, but it was missing a few key items:

Schema information (as in when did columns appear / disappear in the plan)
A visual representation (graphviz)

Open questions:

Would it be better to split the visitor into visitor.rs and display code into display.rs? I am torn -- this is all logically part of logical_plan, but the module is getting kind of big.

Changes:

This PR adds several additional formatting options to logical plans in addition to the existing indent. Examples are included below

To do so it also provides a generalized "Visitor" pattern for walking logical plan nodes, as well as a general pattern to display logical plan nodes with multiple potential formats.

Note it should be straight forward to get this wired up into EXPALIN as well: https://issues.apache.org/jira/browse/ARROW-9746

Existing Formatting

Here is what master currently allows:

Projection: #id
   Filter: #state Eq Utf8(\"CO\")\
       CsvScan: employee.csv projection=Some([0, 3])

With Schema Information.

This PR adds a dump with schema information:

 Projection: #id [id:Int32]\
    Filter: #state Eq Utf8(\"CO\") [id:Int32, state:Utf8]\
      TableScan: employee.csv projection=Some([0, 3]) [id:Int32, state:Utf8]";

As Graphviz

This PR adds the ability to display plans using Graphviz

Here is an example GraphViz plan that comes out:

// Begin DataFusion GraphViz Plan (see https://graphviz.org)
digraph {
  subgraph cluster_1
  {
    graph[label="LogicalPlan"]
    2[label="Projection: #id"]
    3[label="Filter: #state Eq Utf8(_CO_)"]
    2 -> 3 [arrowhead=none, arrowtail=normal, dir=back]
    4[label="TableScan: employee.csv projection=Some([0, 3])"]
    3 -> 4 [arrowhead=none, arrowtail=normal, dir=back]
  }
  subgraph cluster_5
  {
    graph[label="Detailed LogicalPlan"]
    6[label="Projection: #id\nSchema: [id:Int32]"]
    7[label="Filter: #state Eq Utf8(_CO_)\nSchema: [id:Int32, state:Utf8]"]
    6 -> 7 [arrowhead=none, arrowtail=normal, dir=back]
    8[label="TableScan: employee.csv projection=Some([0, 3])\nSchema: [id:Int32, state:Utf8]"]
    7 -> 8 [arrowhead=none, arrowtail=normal, dir=back]
  }
}
// End DataFusion GraphViz Plan

Here is what that looks like rendered:

alamb · 2020-11-09T22:59:45Z

I am sorry for the PR size -- but it is mostly comments and tests

alamb · 2020-11-09T23:01:15Z

rust/datafusion/src/logical_plan/mod.rs

+/// ```
+///
+/// Example use: TODO
+pub trait PlanVisitor {


I actually think this introduction is the most important thing about this PR -- I would like to move most of the recursive walks of LogicalPlans to be in terms of such a structure.

alamb · 2020-11-09T23:01:41Z

rust/datafusion/src/logical_plan/mod.rs

+///      format!("{}", display_schema(&schema))
+///  );
+/// ```
+pub fn display_schema<'a>(schema: &'a Schema) -> impl fmt::Display + 'a {


I wonder if this belongs in arrow rather than DataFusion? I could go either way

github-actions · 2020-11-09T23:05:26Z

https://issues.apache.org/jira/browse/ARROW-10531

andygrove · 2020-11-10T15:34:06Z

rust/datafusion/src/logical_plan/mod.rs

+
+        writeln!(
+            self.f,
+            "    {}[label={}]",


I think the plans would look better and use less space if we added shape=box here.

Agreed -- updated. The plans now look like this:

andygrove · 2020-11-10T15:37:44Z

Thanks @alamb. I am also a fan of using GraphViz to render query plans so this gets a 👍 from me. I will try and find time to review fully later today.

jorgecarleitao

This is super cool!

I went through this and it looks good to me 👍

What do you think of adding instructions on how to create the graphs in the README, or as an example? Atm it feels that this feature may not be utilized due to its unknown to people.

A minor concern is the file size: I would be supportive of moving the visitor to a different file, to avoid exploding this one even further. A later PR is also fine, though :P

alamb · 2020-11-10T23:13:17Z

@jorgecarleitao

What do you think of adding instructions on how to create the graphs in the README, or as an example?

Great idea. I have added some instructions in b1965fa

A minor concern is the file size: I would be supportive of moving the visitor to a different file, to avoid exploding this one even further. A later PR is also fine, though :P

I will do so as a follow on PR after this is merged

@andygrove -- I will wait for your review / feedback before doing anything more with this PR.

andygrove

LGTM. Thanks @alamb

alamb · 2020-11-11T13:48:37Z

Thanks @andygrove and @jorgecarleitao -- I plan to merge this PR and then make a new one to break the code into smaller modules

…ith schema and graphviz, and a PlanVisitor

alamb · 2020-11-11T13:50:10Z

Rebased and will merge when it passes CI

alamb · 2020-11-11T16:13:23Z

#8639 is the PR for breaking up the logical_plan module

… modules The module has gotten fairly large and so refactoring it into smaller chunks will improve readability – as suggested by Jorge #8619 (review) This PR just moves code around -- it is not intended to change any semantics Reviewing it commit-by-commit might be helpful to see how each piece went I can also break it up into a sequence of smaller PRs if that would help review Closes #8639 from alamb/alamb/ARROW-10559-split-up Authored-by: alamb <andrew@nerdnetworks.org> Signed-off-by: Jorge C. Leitao <jorgecarleitao@gmail.com>

…or LogicalPlans and a PlanVisitor # Rationale: I have been tracking down potential issues DataFusion for my work project, and I have found myself wanting to print out the state of the logical_plan several times. The existing debug formatting is ok, but it was missing a few key items: 1. Schema information (as in when did columns appear / disappear in the plan) 2. A visual representation (graphviz) # Open questions: 1. Would it be better to split the visitor into `visitor.rs` and display code into `display.rs`? I am torn -- this is all logically part of logical_plan, but the module is getting kind of big. # Changes: This PR adds several additional formatting options to logical plans in addition to the existing indent. Examples are included below To do so it also provides a generalized "Visitor" pattern for walking logical plan nodes, as well as a general pattern to display logical plan nodes with multiple potential formats. Note it should be straight forward to get this wired up into EXPALIN as well: https://issues.apache.org/jira/browse/ARROW-9746 ## Existing Formatting Here is what master currently allows: ``` Projection: #id Filter: #state Eq Utf8(\"CO\")\ CsvScan: employee.csv projection=Some([0, 3]) ``` ## With Schema Information. This PR adds a dump with schema information: ``` Projection: #id [id:Int32]\ Filter: #state Eq Utf8(\"CO\") [id:Int32, state:Utf8]\ TableScan: employee.csv projection=Some([0, 3]) [id:Int32, state:Utf8]"; ``` ## As Graphviz This PR adds the ability to display plans using [Graphviz](http://www.graphviz.org) Here is an example GraphViz plan that comes out: ``` // Begin DataFusion GraphViz Plan (see https://graphviz.org) digraph { subgraph cluster_1 { graph[label="LogicalPlan"] 2[label="Projection: #id"] 3[label="Filter: #state Eq Utf8(_CO_)"] 2 -> 3 [arrowhead=none, arrowtail=normal, dir=back] 4[label="TableScan: employee.csv projection=Some([0, 3])"] 3 -> 4 [arrowhead=none, arrowtail=normal, dir=back] } subgraph cluster_5 { graph[label="Detailed LogicalPlan"] 6[label="Projection: #id\nSchema: [id:Int32]"] 7[label="Filter: #state Eq Utf8(_CO_)\nSchema: [id:Int32, state:Utf8]"] 6 -> 7 [arrowhead=none, arrowtail=normal, dir=back] 8[label="TableScan: employee.csv projection=Some([0, 3])\nSchema: [id:Int32, state:Utf8]"] 7 -> 8 [arrowhead=none, arrowtail=normal, dir=back] } } // End DataFusion GraphViz Plan ``` Here is what that looks like rendered: <img width="1679" alt="Screen Shot 2020-11-09 at 2 30 07 PM" src="https://user-images.githubusercontent.com/490673/98606322-0f891880-22b5-11eb-8e1c-669ce85f0f52.png"> Closes apache#8619 from alamb/alamb/improved-display Authored-by: alamb <andrew@nerdnetworks.org> Signed-off-by: alamb <andrew@nerdnetworks.org>

… modules The module has gotten fairly large and so refactoring it into smaller chunks will improve readability – as suggested by Jorge apache#8619 (review) This PR just moves code around -- it is not intended to change any semantics Reviewing it commit-by-commit might be helpful to see how each piece went I can also break it up into a sequence of smaller PRs if that would help review Closes apache#8639 from alamb/alamb/ARROW-10559-split-up Authored-by: alamb <andrew@nerdnetworks.org> Signed-off-by: Jorge C. Leitao <jorgecarleitao@gmail.com>

…or LogicalPlans and a PlanVisitor # Rationale: I have been tracking down potential issues DataFusion for my work project, and I have found myself wanting to print out the state of the logical_plan several times. The existing debug formatting is ok, but it was missing a few key items: 1. Schema information (as in when did columns appear / disappear in the plan) 2. A visual representation (graphviz) # Open questions: 1. Would it be better to split the visitor into `visitor.rs` and display code into `display.rs`? I am torn -- this is all logically part of logical_plan, but the module is getting kind of big. # Changes: This PR adds several additional formatting options to logical plans in addition to the existing indent. Examples are included below To do so it also provides a generalized "Visitor" pattern for walking logical plan nodes, as well as a general pattern to display logical plan nodes with multiple potential formats. Note it should be straight forward to get this wired up into EXPALIN as well: https://issues.apache.org/jira/browse/ARROW-9746 ## Existing Formatting Here is what master currently allows: ``` Projection: #id Filter: #state Eq Utf8(\"CO\")\ CsvScan: employee.csv projection=Some([0, 3]) ``` ## With Schema Information. This PR adds a dump with schema information: ``` Projection: #id [id:Int32]\ Filter: #state Eq Utf8(\"CO\") [id:Int32, state:Utf8]\ TableScan: employee.csv projection=Some([0, 3]) [id:Int32, state:Utf8]"; ``` ## As Graphviz This PR adds the ability to display plans using [Graphviz](http://www.graphviz.org) Here is an example GraphViz plan that comes out: ``` // Begin DataFusion GraphViz Plan (see https://graphviz.org) digraph { subgraph cluster_1 { graph[label="LogicalPlan"] 2[label="Projection: #id"] 3[label="Filter: #state Eq Utf8(_CO_)"] 2 -> 3 [arrowhead=none, arrowtail=normal, dir=back] 4[label="TableScan: employee.csv projection=Some([0, 3])"] 3 -> 4 [arrowhead=none, arrowtail=normal, dir=back] } subgraph cluster_5 { graph[label="Detailed LogicalPlan"] 6[label="Projection: #id\nSchema: [id:Int32]"] 7[label="Filter: #state Eq Utf8(_CO_)\nSchema: [id:Int32, state:Utf8]"] 6 -> 7 [arrowhead=none, arrowtail=normal, dir=back] 8[label="TableScan: employee.csv projection=Some([0, 3])\nSchema: [id:Int32, state:Utf8]"] 7 -> 8 [arrowhead=none, arrowtail=normal, dir=back] } } // End DataFusion GraphViz Plan ``` Here is what that looks like rendered: <img width="1679" alt="Screen Shot 2020-11-09 at 2 30 07 PM" src="https://user-images.githubusercontent.com/490673/98606322-0f891880-22b5-11eb-8e1c-669ce85f0f52.png"> Closes apache#8619 from alamb/alamb/improved-display Authored-by: alamb <andrew@nerdnetworks.org> Signed-off-by: alamb <andrew@nerdnetworks.org>

… modules The module has gotten fairly large and so refactoring it into smaller chunks will improve readability – as suggested by Jorge apache#8619 (review) This PR just moves code around -- it is not intended to change any semantics Reviewing it commit-by-commit might be helpful to see how each piece went I can also break it up into a sequence of smaller PRs if that would help review Closes apache#8639 from alamb/alamb/ARROW-10559-split-up Authored-by: alamb <andrew@nerdnetworks.org> Signed-off-by: Jorge C. Leitao <jorgecarleitao@gmail.com>

github-actions bot added Component: Rust - DataFusion Component: Rust labels Nov 9, 2020

alamb commented Nov 9, 2020

View reviewed changes

andygrove reviewed Nov 10, 2020

View reviewed changes

jorgecarleitao approved these changes Nov 10, 2020

View reviewed changes

andygrove approved these changes Nov 11, 2020

View reviewed changes

alamb added 4 commits November 11, 2020 08:49

ARROW-10531: [Rust][DataFusion]: Add multiple LogicalPlan displays: w…

a437c04

…ith schema and graphviz, and a PlanVisitor

cleanup comments

9ef91a9

fix: Use shape=box

c5d8abe

Add note about how to render graphviz into the README

44b9f8f

alamb force-pushed the alamb/improved-display branch from b1965fa to 44b9f8f Compare November 11, 2020 13:49

alamb closed this in 8e4d9eb Nov 11, 2020

alamb mentioned this pull request Nov 11, 2020

ARROW-10559: [Rust][DataFusion] Split up logical_plan/mod.rs into sub modules #8639

Closed

alamb deleted the alamb/improved-display branch December 7, 2020 19:30

This was referenced Nov 11, 2020

[Rust] [DataFusion] Better display for logical plans: Graphviz and Schema information #26500

Closed

[Rust] [DataFusion] Break up logical_plan/mod.rs into smaller modules #26524

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ARROW-10531: [Rust][DataFusion]: Add schema and graphviz formatting for LogicalPlans and a PlanVisitor #8619

ARROW-10531: [Rust][DataFusion]: Add schema and graphviz formatting for LogicalPlans and a PlanVisitor #8619

alamb commented Nov 9, 2020

alamb commented Nov 9, 2020

alamb Nov 9, 2020

alamb Nov 9, 2020

github-actions bot commented Nov 9, 2020

andygrove Nov 10, 2020

alamb Nov 10, 2020

andygrove commented Nov 10, 2020

jorgecarleitao left a comment

alamb commented Nov 10, 2020

andygrove left a comment

alamb commented Nov 11, 2020

alamb commented Nov 11, 2020

alamb commented Nov 11, 2020

ARROW-10531: [Rust][DataFusion]: Add schema and graphviz formatting for LogicalPlans and a PlanVisitor #8619

ARROW-10531: [Rust][DataFusion]: Add schema and graphviz formatting for LogicalPlans and a PlanVisitor #8619

Conversation

alamb commented Nov 9, 2020

Rationale:

Open questions:

Changes:

Existing Formatting

With Schema Information.

As Graphviz

alamb commented Nov 9, 2020

alamb Nov 9, 2020

Choose a reason for hiding this comment

alamb Nov 9, 2020

Choose a reason for hiding this comment

github-actions bot commented Nov 9, 2020

andygrove Nov 10, 2020

Choose a reason for hiding this comment

alamb Nov 10, 2020

Choose a reason for hiding this comment

andygrove commented Nov 10, 2020

jorgecarleitao left a comment

Choose a reason for hiding this comment

alamb commented Nov 10, 2020

andygrove left a comment

Choose a reason for hiding this comment

alamb commented Nov 11, 2020

alamb commented Nov 11, 2020

alamb commented Nov 11, 2020