metegenez commented on code in PR #1649:
URL:
https://github.com/apache/datafusion-ballista/pull/1649#discussion_r3204246017
##########
ballista/scheduler/src/state/aqe/optimizer_rule/distributed_exchange.rs:
##########
@@ -138,3 +130,515 @@ impl PhysicalOptimizerRule for DistributedExchangeRule {
false
}
}
+
+/// Scans the subtree for the nearest `ExchangeExec` in each path and returns
the
+/// aggregate status. Stops recursing at `ExchangeExec` boundaries so that
only the
+/// shallowest exchange in each branch is considered.
+///
+/// Returns `Unresolved` as soon as any branch contains an unresolved exchange
+/// (short-circuits), `Resolved` if every branch that has an exchange has a
resolved
+/// one, and `None` if no exchange is found anywhere.
+fn find_exchange_status(plan: &Arc<dyn ExecutionPlan>) -> ExchangeStatus {
Review Comment:
We can make this method's name 'nearest_exchange_status_per_branch ', which
is easier for non-active people to understand.
##########
ballista/scheduler/src/state/aqe/optimizer_rule/distributed_exchange.rs:
##########
@@ -25,82 +25,74 @@ use datafusion::physical_plan::{ExecutionPlan,
execution_plan};
use std::sync::Arc;
use std::sync::atomic::AtomicUsize;
+enum ExchangeStatus {
+ None,
Review Comment:
very small, but naming the 'no exchange anywhere in the subtree' case
something like Absent or NoExchange would make the code easier to maintain.
--
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]
