alamb commented on code in PR #4839:
URL: https://github.com/apache/arrow-datafusion/pull/4839#discussion_r1064123833
##########
datafusion/core/src/physical_optimizer/dist_enforcement.rs:
##########
@@ -1136,8 +1115,10 @@ mod tests {
config.execution.target_partitions = 10;
// run optimizer
- let optimizer = BasicEnforcement {};
+ let optimizer = EnforceDistribution {};
let optimized = optimizer.optimize($PLAN, &config)?;
+ let optimizer = EnforceSorting {};
Review Comment:
it is slightly confusing that the tests in `EnforceDistribution` also rely
on `EnforceSorting` though I understand the reason for this given they both
started in the same pass
Maybe we can add a comment like:
```
// These tests also ensure `EnforceSorting` because they were written prior
to the
// separation of `EnforceSorting` and `EnfoceDistribution`
```
Or something like that to give future readers a clue about the rationale for
this seemingly strange choice
##########
datafusion/core/src/physical_plan/limit.rs:
##########
@@ -287,6 +287,10 @@ impl ExecutionPlan for LocalLimitExec {
self.input.output_ordering()
}
+ fn maintains_input_order(&self) -> bool {
Review Comment:
👍
--
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]
