[GitHub] [arrow] alamb commented on pull request #8619: ARROW-10531: [Rust][DataFusion]: Add schema and graphviz formatting for LogicalPlans and a PlanVisitor

2020-11-11 Thread GitBox 


alamb commented on pull request #8619:
URL: https://github.com/apache/arrow/pull/8619#issuecomment-725513681


   https://github.com/apache/arrow/pull/8639 is the PR for breaking up the 
logical_plan module



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [arrow] alamb commented on pull request #8619: ARROW-10531: [Rust][DataFusion]: Add schema and graphviz formatting for LogicalPlans and a PlanVisitor

2020-11-11 Thread GitBox 


alamb commented on pull request #8619:
URL: https://github.com/apache/arrow/pull/8619#issuecomment-725433951


   Rebased and will merge when it passes CI



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [arrow] alamb commented on pull request #8619: ARROW-10531: [Rust][DataFusion]: Add schema and graphviz formatting for LogicalPlans and a PlanVisitor

2020-11-11 Thread GitBox 


alamb commented on pull request #8619:
URL: https://github.com/apache/arrow/pull/8619#issuecomment-725433229


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



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [arrow] alamb commented on pull request #8619: ARROW-10531: [Rust][DataFusion]: Add schema and graphviz formatting for LogicalPlans and a PlanVisitor

2020-11-10 Thread GitBox 


alamb commented on pull request #8619:
URL: https://github.com/apache/arrow/pull/8619#issuecomment-725023738


   @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 
https://github.com/apache/arrow/pull/8619/commits/b1965fae167ac46ff399752c55a8728b78de2bca
   
   > 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.



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [arrow] alamb commented on pull request #8619: ARROW-10531: [Rust][DataFusion]: Add schema and graphviz formatting for LogicalPlans and a PlanVisitor

2020-11-09 Thread GitBox 


alamb commented on pull request #8619:
URL: https://github.com/apache/arrow/pull/8619#issuecomment-724330317


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



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org