mustafasrepo commented on code in PR #7488:
URL: https://github.com/apache/arrow-datafusion/pull/7488#discussion_r1319488427
##########
datafusion/core/src/physical_plan/repartition/mod.rs:
##########
@@ -637,7 +637,8 @@ impl RepartitionExec {
/// Set Order preserving flag
pub fn with_preserve_order(mut self, preserve_order: bool) -> Self {
- self.preserve_order = preserve_order;
+ // Set "preserve order" mode only if the operator cannot maintain the
order:
+ self.preserve_order = preserve_order &&
!self.maintains_input_order()[0];
Review Comment:
That would make it clear. The reason for this change is that,
`RepartitionExec` and `SortPreservingRepartitionExec` has different approaches.
`SortPreservingRepartitionExec` adds additional overhead (`preserve_order` flag
controls these modes, if `true` mode is `SortPreserving`. However, when input
partition number is 1, `RepartitionExec` can also maintain ordering. Hence for
this case, we do not need to use `SortPreservingRepartitionExec`). I agree that
your proposed version is more readable. I changed the implementation according
to your suggestion
--
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]
