alamb commented on code in PR #10259:
URL: https://github.com/apache/datafusion/pull/10259#discussion_r1581819981
##########
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
macro_rules! assert_optimized {
($EXPECTED_LINES: expr, $PLAN: expr, $FIRST_ENFORCE_DIST: expr) => {
- assert_optimized!($EXPECTED_LINES, $PLAN, $FIRST_ENFORCE_DIST,
false, 10, false, 1024);
+ assert_optimized!($EXPECTED_LINES, $PLAN, $FIRST_ENFORCE_DIST,
false, 10, false, 1024, false);
};
($EXPECTED_LINES: expr, $PLAN: expr, $FIRST_ENFORCE_DIST: expr,
$PREFER_EXISTING_SORT: expr) => {
- assert_optimized!($EXPECTED_LINES, $PLAN, $FIRST_ENFORCE_DIST,
$PREFER_EXISTING_SORT, 10, false, 1024);
+ assert_optimized!($EXPECTED_LINES, $PLAN, $FIRST_ENFORCE_DIST,
$PREFER_EXISTING_SORT, 10, false, 1024, false);
Review Comment:
these macros are getting a bit hairy -- maybe we can clean them up (convert
to using functions rather than macros) in a subsequent PR
##########
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
([influxdata#4](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 excellent point @phillipleblanc . I just double checked and we
actually have `prefer_existing_sort` set to true in IOx already ([code ref, not
public](https://github.com/influxdata/influxdb_iox/blob/ec0b64557a70fc262ca09e6a6c941ae642667f31/datafusion_util/src/config.rs#L34))
What do you think about using the existing flag @NGA-TRAN ?
--
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
