Stop copying LogicalPlan and Exprs in EliminateNestedUnion

[emgeee]

Which issue does this PR close?

Closes #10296.

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Existing tests pass

Are there any user-facing changes?

[emgeee]

In addition to updating the interface for this optimizer, I tried to remove as many copies as possible but I'm still new to rust + datafusion so I'm very much open to suggestions and feedback!

[alamb]

Nice! Thank you @emgeee

I think this is a step forward for sure

One thing I noticed is that

fn extract_plans_from_union(plan: &Arc<LogicalPlan>) -> Vec<Arc<LogicalPlan>> {
    match plan.as_ref() {
        LogicalPlan::Union(Union { inputs, schema }) => inputs
            .iter()
            .map(|plan| Arc::new(coerce_plan_expr_for_schema(plan, schema).unwrap()))
            .collect::<Vec<_>>(),
        _ => vec![plan.clone()],
    }
}

Calls coerce_plan_expr_for_schema which looks like it may still clone the input and exprs.

Maybe we can try to get rid of those clones too (but as a follow on PR)

[alamb]

Marking as draft as I think this PR is no longer waiting on feedback. Please mark it as ready for review when it is ready for another look

[alamb]

(also looks like there are some tests that need to be fixed)

[emgeee]

I went ahead and fixed the formatting so all tests pass and managed to remove 1 clone() call from with in the coerce_plan_expr_for_schema() call stack.

If we want to optimize coerce_plan_expr_for_schema further, I'd definitely agree a separate PR makes sense as it is called from a number of other locations as well

[alamb]

Thanks you for this contribution @emgeee -- a very nice first contribution .

If we want to optimize coerce_plan_expr_for_schema further, I'd definitely agree a separate PR makes sense as it is called from a number of other locations as well

I agree -- this would be great to remove additional copies. If you had the time PRs would be most appreciated 🙏

[alamb]

🚀

[alamb]

Thanks again @emgeee