Implement LogicalPlanBuilder::from for Arc<LogicalPlan>

[alamb]

For historical reasons children in LogicalPlan are Arc<LogicalPlan> (so shared, immutable references)

For many optimizer optimizations we need a owned LogicalPlan

This can be achieved via unwrap_arc

However, as pointed out by #10460 (comment) @ClSlaid it is not obvious at first

Today to get a builder from a plan you need to know this pattern:

// the builder accepts LogicalPlan only, so get one via unwrap_arc
let mut builder = LogicalPlanBuilder::from(unwrap_arc(plan));

I think it would be great to support this pattern:

// the builder accepts Arc<LogicalPlan> directly
let mut builder = LogicalPlanBuilder::from(plan);

And the underlying impl From<Arc<LogicalPlan>> for LogicalPlanBuilder would call unwrap_arc

[ClSlaid]

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.

[alamb]

That is an interesting point @ClSlaid

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

[ClSlaid]

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.

[alamb]

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

datafusion/datafusion/expr/src/logical_plan/builder.rs

Lines 98 to 101 in 8cc92a9

	#[derive(Debug, Clone)]
	pub struct LogicalPlanBuilder {
	plan: LogicalPlan,
	}

To

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

[alamb]

Filed #10485 to track the internal representation change