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 `RangePartitioning::scale` to be preferable to something that has 
been hashed... but right now I don't think that that makes sense.
   
   So probably `scaled_native_range` should be removed entirely.



-- 
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]

Reply via email to