Improve documentation

datafusion/core/src/physical_optimizer/repartition.rs

@@ -129,10 +129,11 @@ impl Repartition {
     }
 }
 
-/// Recursively visits all `plan`s puts and then optionally adds a
-/// `RepartitionExec` at the output of `plan` to match
-/// `target_partitions` in an attempt to increase the overall
-/// parallelism while respecting ordering that exists within the plan.
+/// Attempt to increase the overall parallelism of the plan, while
+/// respecting ordering that exists. Recursively visits all `plan`s
+/// inputs, optionally adding a `RepartitionExec` at the output of
+/// `plan` if it would help and does not destroy any required
+/// ordering.
 ///
 /// It does so using depth first scan of the tree, and repartitions
 /// any plan that:
@@ -143,11 +144,10 @@ impl Repartition {
 ///
 /// if `can_reorder` is true, it means the output of `plan` can be
 /// reordered without affecting nodes above it. It can_reorder is
-/// false, it, means that the parent node of this node is relying on
-/// the order
+/// false, it means some node above this node is relying on the order.
 ///
-/// If 'would_benefit` is false, the upstream operator doesn't
-///  benefit from additional repartition
+/// If 'would_benefit` is true, the upstream operator would benefit
+/// from additional partitions and thus repatitioning is considered.
 ///
 fn optimize_partitions(
     target_partitions: usize,

datafusion/core/tests/sql/window.rs

@@ -2311,9 +2311,9 @@ async fn test_remove_unnecessary_sort_in_sub_query() -> Result<()> {
 
     let actual: Vec<&str> = formatted.trim().lines().collect();
     let actual_len = actual.len();
-    let actual_trim_last = &actual[..actual_len - 1];
+    //let actual_trim_last = &actual[..actual_len - 1];
     assert_eq!(
-        expected, actual_trim_last,
+        expected, actual,
         "\n\nexpected:\n\n{expected:#?}\nactual:\n\n{actual:#?}\n\n"
     );