avantgardnerio commented on code in PR #2196:
URL:
https://github.com/apache/datafusion-ballista/pull/2196#discussion_r3696975559
##########
ballista/scheduler/src/state/aqe/optimizer_rule/distributed_exchange.rs:
##########
@@ -90,11 +93,36 @@ impl DistributedExchangeRule {
);
return Ok(Transformed::yes(Arc::new(exchange_exec)));
}
+ } else if !execution_plan.is::<ExchangeExec>()
+ && let [child] = execution_plan.children().as_slice()
Review Comment:
Thanks @andygrove . The change was intentional, though I did a poor job
explaining that. Rationale: if this does both sides of a join (for instance),
the scheduler needs to merge the cuts _from both sides of the join_ instead of
each leg independently. I have not written that code yet, so walking both sides
(like it did before) would have introduced another silent bug.
SMJ is definitely an intended use-case, so I think we need to add it in a
follow up PR, but it is not a blocker for windows, so this PR does not include
it - to keep this PR from growing further.
So I took option B from your advice and documented the behavior. The PR
title probably slightly over-claims now, but I hope now that the code is
consistent we can merge.
WDYT?
--
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]
