yahoNanJing commented on code in PR #4714:
URL: https://github.com/apache/arrow-datafusion/pull/4714#discussion_r1057038413
##########
datafusion/core/src/physical_optimizer/repartition.rs:
##########
@@ -546,12 +538,12 @@ mod tests {
}
#[test]
- fn repartition_ignores_sort_preserving_merge() -> Result<()> {
+ fn repartition_with_preserving_merge() -> Result<()> {
let plan = sort_preserving_merge_exec(parquet_exec());
let expected = &[
"SortPreservingMergeExec: [c1@0 ASC]",
- // Expect no repartition of SortPreservingMergeExec
+ "RepartitionExec: partitioning=RoundRobinBatch(10)",
Review Comment:
Hi @Dandandan, actually, the UT here is not for the whole optimizing
process, since the manually constructed plan is not a real case. The real case
should insert a parallel local sort as the input of `SortPreservingMergeExec`,
which is good for the round robin `RepartitionExec` between the `ParquetExec`
and the `SortExec`.
What's more, current rule of `RepartitionExec` employs a bottom-up policy
rather than the top-down policy, which makes it hard to consider whether to
insert a `RepartitionExec` as an input of some operator.
--
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]
