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