alamb commented on a change in pull request #744:
URL: https://github.com/apache/arrow-datafusion/pull/744#discussion_r671827286



##########
File path: datafusion/tests/sql.rs
##########
@@ -47,6 +47,41 @@ use datafusion::{
 };
 use datafusion::{execution::context::ExecutionContext, 
physical_plan::displayable};
 
+/// Copied from datafusion/test/mod.rs
+/// TODO: consolidate

Review comment:
       This is basically a copy/paste from test/mod.rs -- I have plans to fix 
this in https://github.com/apache/arrow-datafusion/issues/743

##########
File path: datafusion/src/physical_plan/planner.rs
##########
@@ -749,32 +754,9 @@ impl DefaultPhysicalPlanner {
                     "Unsupported logical plan: 
CreateExternalTable".to_string(),
                 ))
             }
-            LogicalPlan::Explain {

Review comment:
       previously the physical planner could handle an `LogicalPlan::Explain` 
anywhere in the tree. However, I can't think if cases where this can actually 
occur and it made adding the final physical plan challenging. 
   
   So I have changed the code to special case handling `Explain` as the root.




-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to