stuhood commented on code in PR #24766:
URL: https://github.com/apache/datafusion/pull/24766#discussion_r4067528351
##########
datafusion/physical-optimizer/src/ensure_requirements/enforce_distribution.rs:
##########
@@ -1154,7 +1157,8 @@ fn enforce_distribution_relationships(
let partitioning = plan.output_partitioning();
match partitioning {
Partitioning::Range(_) | Partitioning::Hash(_, _) => {
- let is_native = !plan.is::<RepartitionExec>();
+ let is_native =
+ !plan.is::<RepartitionExec>() ||
child.scaled_native_range;
Review Comment:
We shouldn't treat a node which has been scaled with `RepartitionExec` as
"natively partitioned": this pass is about finding sources which already
satisfy the constraints, without the addition of a `RepartitionExec`.
Unless a node can be scaled with `ExecutionPlan::repartitioned`, we
shouldn't consider it to be natively partitioned here, as that will prioritize
adapting some other data source to match this one.
It's possible that after #25483 we might consider a `Range` which has been
scaled with `Range` to be preferable to something that has been hashed... but
right now I don't think that makes sense.
So probably `scaled_native_range` should be removed entirely.
##########
datafusion/physical-optimizer/src/ensure_requirements/enforce_distribution.rs:
##########
@@ -1194,11 +1198,9 @@ fn enforce_distribution_relationships(
})
.collect();
- // Only select a reference candidate if there is a unique,
strictly
- // larger winner (`size_a > size_b`). If candidates have
equal or
- // incomparable sizes (e.g. non-overlapping metrics),
return None
- // so the optimizer avoids arbitrary tie-breaking and
falls back to
- // standard distribution.
+ // Prefer a unique, strictly larger winner (`size_a >
size_b`).
+ // Otherwise avoid arbitrary tie-breaking, except for the
new
+ // preserved-range case below.
Review Comment:
Stale I think.
--
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]
