alamb commented on code in PR #11751:
URL: https://github.com/apache/datafusion/pull/11751#discussion_r1698920706


##########
datafusion/core/src/physical_optimizer/enforce_sorting.rs:
##########
@@ -857,14 +852,14 @@ mod tests {
     #[tokio::test]
     async fn test_remove_unnecessary_sort1() -> Result<()> {
         let schema = create_test_schema()?;
-        let source = memory_exec(&schema);
         let sort_exprs = vec![sort_expr("nullable_col", &schema)];
-        let sort = sort_exec(sort_exprs.clone(), source);
-        let spm = sort_preserving_merge_exec(sort_exprs, sort);
+        let physical_plan = TestPlanBuilder::new_memory_exec(&schema)
+            .sort_by_exprs(sort_exprs.clone())
+            .sort_preserving_merge(sort_exprs.clone())
+            .sort_by_exprs(sort_exprs.clone())
+            .sort_preserving_merge(sort_exprs)
+            .build();
 
-        let sort_exprs = vec![sort_expr("nullable_col", &schema)];
-        let sort = sort_exec(sort_exprs.clone(), spm);
-        let physical_plan = sort_preserving_merge_exec(sort_exprs, sort);
         let expected_input = [
             "SortPreservingMergeExec: [nullable_col@0 ASC]",

Review Comment:
   The tests already have an exepcted output that nicely confirms the created 
plan still has the same structure as the previous tests



##########
datafusion/core/src/physical_optimizer/enforce_sorting.rs:
##########
@@ -775,43 +776,39 @@ mod tests {
     #[tokio::test]
     async fn test_remove_unnecessary_sort_window_multilayer() -> Result<()> {
         let schema = create_test_schema()?;
-        let source = memory_exec(&schema);
 
         let sort_exprs = vec![sort_expr_options(
             "non_nullable_col",
-            &source.schema(),
+            &schema,
             SortOptions {
                 descending: true,
                 nulls_first: true,
             },
         )];
-        let sort = sort_exec(sort_exprs.clone(), source);
-        // Add dummy layer propagating Sort above, to test whether sort can be 
removed from multi layer before
-        let coalesce_batches = coalesce_batches_exec(sort);
 
-        let window_agg =
-            bounded_window_exec("non_nullable_col", sort_exprs, 
coalesce_batches);
+        let builder = TestPlanBuilder::new_memory_exec(&schema)

Review Comment:
   This shows a good example of the difference -- the plan gets built in 
builder chain rather than several individual function calls



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to