thinkharderdev commented on PR #3677:
URL: 
https://github.com/apache/arrow-datafusion/pull/3677#issuecomment-1264414073

   > I like the idea of supplementing the existing trait instead of replacing 
it, I in fact suggested something very similar back in April - [#2175 
(comment)](https://github.com/apache/arrow-datafusion/issues/2175#issuecomment-1094950365)
   > 
   > I would perhaps suggest using borrows of the concrete type vs cloned Arc, 
I think this will make for a cleaner interface that is easier to work with.
   > 
   > So something like
   > 
   > ```
   > pub enum ConcretePhysicalPlan<'a> {
   >     // TODO add all of DataFusion's operators
   >     Projection(&'a ProjectionExec),
   >     Filter(&'a FilterExec),
   >     HashJoin(&'a HashJoinExec),
   >     Custom(&'a dyn ExecutionPlan),
   > }
   > 
   > impl <'a> ConcretePhysicalPlan<'a> {
   >     fn downcast(plan: &'a dyn ExecutionPlan) -> Self {
   >         // Long list of if statements, or possibly something more 
sophisticated
   >         ...
   >     }
   > }
   > ```
   
   Could we do something like
   
   ```rust
   trait ExecutionPlan {
     ....
   
     fn lift(&self) -> ConcretePhysicalPlan<'_>;
   }
   
   impl ExecutionPlan for ProjectionExec {
     fn lift(&self) -> ConcretePhysicalPlan<'_> {
       ConcretePhysicalPlan::Projection(self)
     }
   }
   
   pub enum ConcretePhysicalPlan<'a> {
       // TODO add all of DataFusion's operators
       Projection(&'a ProjectionExec),
       Filter(&'a FilterExec),
       HashJoin(&'a HashJoinExec),
       Custom(&'a dyn ExecutionPlan),
   }
   ```


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