findepi commented on issue #13202: URL: https://github.com/apache/datafusion/issues/13202#issuecomment-2450181549
> 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: > > ```rust > 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. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
