phillipleblanc commented on code in PR #10259: URL: https://github.com/apache/datafusion/pull/10259#discussion_r1581741974
########## datafusion/core/src/physical_optimizer/enforce_distribution.rs: ########## @@ -1723,21 +1727,30 @@ pub(crate) mod tests { /// * `REPARTITION_FILE_MIN_SIZE` (optional) - minimum file size to repartition Review Comment: ```suggestion /// * `REPARTITION_FILE_MIN_SIZE` (optional) - minimum file size to repartition /// * `PREFER_EXISTING_UNION` (optional) - if true, will not attempt to convert Union to Interleave ``` ########## datafusion/core/src/physical_optimizer/enforce_distribution.rs: ########## @@ -1192,7 +1192,11 @@ fn ensure_distribution( .collect::<Result<Vec<_>>>()?; let children_plans = children.iter().map(|c| c.plan.clone()).collect::<Vec<_>>(); - plan = if plan.as_any().is::<UnionExec>() && can_interleave(children_plans.iter()) { + + plan = if plan.as_any().is::<UnionExec>() + && !config.optimizer.prefer_existing_union Review Comment: Given that one of the motivations (https://github.com/influxdata/arrow-datafusion/pull/4) for this new flag is to preserve the sorting of the Union - would re-using the existing flag `prefer_existing_sort` make sense here? One argument against that would be if you wanted `prefer_existing_sort: false` and `prefer_existing_union: true`. -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org