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


##########
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:
   In order to be a little easier to read, we can use 
`self.sort_keys.as_deref()` here. This would avoid the construction of a 
single-element vector and then taking the first element of it. The downside of 
doing this would be replicating this line from `required_input_ordering`, but I 
think the added clarity is worth it here. Thoughts, @alamb?



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

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

Reply via email to