Implement `LogicalPlanBuilder::from` for `Arc<LogicalPlan>` about arrow-datafusion HOT 5 CLOSED

Great idea @ClSlaid

Thanks to @AbrarNitk we have a first version of LogicalPlanBuilder::from(arc_input) in #10466 🙏

I think we should merge that PR and then make a second PR (and maybe a second ticket) about switching the internal representation from

To

 pub struct LogicalPlanBuilder { 
     plan: Arc<LogicalPlan>, 
 } 

from arrow-datafusion.

I suggest not call unwrap_arc until the built LogicalPlan really requires the ownership of a LogicalPlan.

In most occasions, LogicalPlan contains an input: Arc<LogicalPlan>, and sometimes we will have to construct LogicalPlanBuilder from it. This will lead to a clone, or (99 percent likely) avoiding that with unwrap_arc.

Meanwhile, most of the LogicalPlans constructed by the builder place the plan in Arcs again. This then causes another clone.

There must be a way to seamlessly moving the input: Arc<LogicalPlan> into rewritten plans, and it would be no clone in most occasions, and at most one clone be committed.

from arrow-datafusion.

That is an interesting point @ClSlaid

So perhaps you are suggesting LogicalPlanBuilder has an Arc<LogicalPlan> as its internal state (rather than a LogicalPlan)

from arrow-datafusion.

So perhaps you are suggesting LogicalPlanBuilder has an Arc<LogicalPlan> as its internal state (rather than a LogicalPlan)

This is surely the most straight forward way.

from arrow-datafusion.

Filed #10485 to track the internal representation change

from arrow-datafusion.