[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #4439: WindowAggExec output ordering is fixed

[GitBox <gi...@apache.org>]

alamb commented on code in PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#discussion_r1036357728


##########
datafusion/core/src/physical_plan/windows/window_agg_exec.rs:
##########
@@ -122,7 +122,9 @@ impl ExecutionPlan for WindowAggExec {
     }
 
     fn output_ordering(&self) -> Option<&[PhysicalSortExpr]> {
-        self.input.output_ordering()
+        // This executor maintains input order, and has required input_ordering filled
+        // hence output_ordering would be `required_input_ordering`
+        self.required_input_ordering()[0]

Review Comment:
   I suggest changing this to `self.input.output_ordering()` to make the intent clearer
   
   I realize that `required_input_ordering()` should produce the same result



-- 
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-unsubscribe@arrow.apache.org

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