Derive Clone for more ExecutionPlans

[alamb]

Is your feature request related to a problem or challenge?

In our code / optimizers we have a lot of code that looks like this when we are
trying to rewrite a SortExec node by making a copy and changing only some of the
fields. For example:

     new_plan = Ok(Arc::new(
                SortExec::new(sort_exec.expr().to_vec(), Arc::clone(sort_exec.input()))
                    .with_fetch(sort_exec.fetch())
                    .with_preserve_partitioning(true),
            ));

This is problematic and we have had bugs in the past when SortExec got a new field (like fetch)

We would like to simply be able to make a copy and modify appropriately:

     new_plan = Ok(Arc::new(sort_exec.clone().with_preserve_partitioning(true)));

Many ExecutionPlans like ParquetExec already implement Clone so it is nice to
make the rest consistent

Describe the solution you'd like

I would like to derive Clone for other plans

Describe alternatives you've considered

No response

Additional context

No response

[findepi]

This is problematic and we have had bugs in the past when SortExec got a new field (like fetch)

We would like to simply be able to make a copy and modify appropriately:

     new_plan = Ok(Arc::new(sort_exec.clone().with_preserve_partitioning(true)));

IMO the right answer in all cases is full destructuring and direct creation, without any shortcuts (like .. or builders).
Why?

When a plan node gains new field, it's impossible to judge apriori about all callsites using this plan node. Does the logic still hold with this new field being added? Does it need to change?
Each callsite needs to be revisited (unless this is printing for debug or whatever).
And the only known way to force this to happen is to not have shortcuts like .. / builders or cloning.

[alamb]

IMO the right answer in all cases is full destructuring and direct creation, without any shortcuts (like .. or builders).

I basically agree with this assesment -- and that is in my mind what a Clone able ExecutionPlan permits

Given the fact that we almost always have Arc<ExecutionPlan>, often the only way to get a full destructing would be with an owned copy

[findepi]

I thought it's an option to destruct taking fields by reference (like in match)

[alamb]

I thought it's an option to destruct taking fields by reference (like in match)

That is true but it won't help modifying the field 🤔

[findepi]

Oh see your point. Destruct by reference, compute new value for a field and then you want to construct new instance. You need shallow clone of all other fields. 👍

[alamb]

Closed in #13203