You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/11/30 10:55:59 UTC

[GitHub] [arrow-datafusion] mustafasrepo opened a new pull request, #4439: WindowAggExec output sort is fixed

mustafasrepo opened a new pull request, #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439

   # Which issue does this PR close?
   
   <!--
   We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
   -->
   
   Closes #4438.
   
   # Rationale for this change
   
   <!--
    Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
    Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.  
   -->
   We can't rely on the current `output_ordering` result unless we pass from the `BasicEnforcement` optimization pass. Hence  `output_ordering` of the `WindowAggExec` would give the wrong result during physical plan [planner](https://github.com/apache/arrow-datafusion/blob/fdc83e8524df30ac5d0ae097572b7c48dc686ba9/datafusion/core/src/physical_plan/planner.rs#L463)
   
   # What changes are included in this PR?
   Since `WindowAggExec` maintains input order and requires specific ordering, output ordering would be required input ordering. I have changed code to reflect that (By this way we can rely on `output_ordering` information of `WindowAggExec` even if there is currently no `SortExec` before it in the physical plan)
   
   
   # Are these changes tested?
   
   N.A
   
   # Are there any user-facing changes?
   
   N.A
   
   <!--
   If there are any breaking changes to public APIs, please add the `api change` label.
   -->


-- 
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


[GitHub] [arrow-datafusion] ozankabak commented on pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
ozankabak commented on PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#issuecomment-1333527426

   Great discussion. I think @metesynnada highlighted the core issue nicely. I don't yet understand the general fix @mingmwang has in mind, but if it works, this PR will become unnecessary.
   
   If it doesn't solve the problem @metesynnada explained, we can use this fix until a final, general fix is available.


-- 
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


[GitHub] [arrow-datafusion] mingmwang commented on pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
mingmwang commented on PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#issuecomment-1333390691

   @mustafasrepo `BasicEnforcement` should do the optimization and avoid the unnecessary SortExec for you. I will write a test in DataFusion late today and try to reproduce the issue.


-- 
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


[GitHub] [arrow-datafusion] metesynnada commented on pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
metesynnada commented on PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#issuecomment-1333481206

   Let's consider more general use cases of `output_ordering()`. In `DefaultPhysicalPlanner.create_initial_plan()`, we should be able to depend on `output_ordering()` and `output_partitioning()` regardless of the optimization step.  Let’s think we have `AExec` and `BExec`.
   
   ```
   AExec
   
     ⬇️
   
   BExec
   ```
   
   where 
   
   ```rust
   impl ExecutionPlan for AExec {
       fn output_ordering(&self) -> Option<&[PhysicalSortExpr]> {
           self.input.output_ordering()
       }
   }
   ```
   
   If I call `AExec.output_ordering()` in `DefaultPhysicalPlanner`, it will gives `BExec`’s output order. I can make decisions depending on it in planner phase. To make this plan “runnable”, we should not add new operators to the between `AExec` and `BExec` outside of the planner. If `AExec` requires an input ordering, this should be decided inside the planner and add to the operator chain to make `output_ordering()` API work. Currently, these API’s are broken since we did not add a `SortExec` inside the planner.


-- 
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


[GitHub] [arrow-datafusion] mingmwang commented on pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
mingmwang commented on PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#issuecomment-1333451112

   > @mingmwang, even though @mustafasrepo's example shows how the issue can manifest in a certain context, the core issue is that `self.input.output_ordering()` can return a result that is inconsistent with `self. required_input_ordering()`. In his specific case, the former returns `None`, which is clearly wrong -- the requirement would have been violated had this been the case.
   > 
   > This seems to happen because as of PR #4122, we insert operators like`SortExec` _after_ physical planning (i.e. during the `BasicEnforcement` optimization step). Therefore, calls like `input_exec.output_ordering()` return such inconsistent/unreliable results during physical planning, since the "real" `input_exec` is not there yet!
   > 
   > Until we figure out a general fix to this, we can select the "finer" ordering between `required_input_ordering()` and `input.output_ordering()` -- this will result in a less wrong result in the meantime.
   
   There is a general fix.  The root cause is not because `output ordering` return None, but when the `WindowAggExec` try to derive the ordering from input, the column index must be handle properly. The same bug exists within output partition.
   
   
   
   I will raise a PR to fix the bug. 


-- 
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


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

Posted by GitBox <gi...@apache.org>.
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-unsubscribe@arrow.apache.org

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


[GitHub] [arrow-datafusion] mingmwang commented on pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
mingmwang commented on PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#issuecomment-1333591038

   The `BasicEnforcement` can choose to satisfy given ordering/partitioning requirements in different ways. For example a requirement ordering `['a'] `can be satisfied by sort by `['a']`, sort by` ['a', 'b']`, order by `['a', 'b', 'c']`, sort by `['a', random]` ......
   In future, we will add support for Range Partitioning, if the optimizer detects the column a is a skewed column,   it can effectively add  sort by `[a + random()] `to avoid data skew in the Range Partitioning.


-- 
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


[GitHub] [arrow-datafusion] mingmwang commented on pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
mingmwang commented on PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#issuecomment-1333572103

   I just submit the fix PR, please help to take a look.
   
   https://github.com/apache/arrow-datafusion/pull/4455


-- 
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


[GitHub] [arrow-datafusion] ozankabak commented on pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
ozankabak commented on PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#issuecomment-1333868609

   The need for this PR was obviated by PR ##4455 thanks to @mingmwang. We will be closing this shortly and focus our efforts on creating a rule that will swap the default `WindowAggExec` with more-efficient, pipeline-able version when appropriate.


-- 
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


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

Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#discussion_r1036371199


##########
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:
   that would also be good 



-- 
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


[GitHub] [arrow-datafusion] mingmwang commented on pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
mingmwang commented on PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#issuecomment-1332926877

   No, I think this change is not correct!!!  WindowAggExec should always keep the input ordering, and the input ordering does not always equal with the required input ordering. For example, input ordering could be `order by ['a', 'b', 'c']`, the required input ordering is `order by ['a', 'b']`,  `order by ['a', 'b', 'c']` can satisfy `order by ['a', 'b']`, in this case, the output ordering of the WindowAggExec itself is still `order by ['a', 'b', 'c']`.
   


-- 
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


[GitHub] [arrow-datafusion] mingmwang commented on pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
mingmwang commented on PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#issuecomment-1333390983

   @mustafasrepo `BasicEnforcement` should do the optimization and avoid the unnecessary SortExec for you. I will write a test in DataFusion late today and try to reproduce the issue.


-- 
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


[GitHub] [arrow-datafusion] ozankabak commented on pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
ozankabak commented on PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#issuecomment-1333430391

   @mingmwang, even though @mustafasrepo's example shows how the issue can manifest in a certain context, the core issue is that `input_exec.output_ordering()` can return a result that is inconsistent with `self. required_input_ordering()`. In his specific case, the former returns `None`, which is clearly wrong -- the requirement would have been violated had this been the case.
   
   This seems to happen because as of PR #4122, we insert operators like`SortExec` *after* physical planning (i.e. during the `BasicEnforcement` optimization step). Therefore, calls like `input_exec.output_ordering()` return such inconsistent/unreliable results during physical planning, since the "real" `input_exec` is not there yet!
   
   Until we figure out a general fix to this, we can select the "finer" ordering between `required_input_ordering()` and `input.output_ordering()` -- this will result in a less wrong result in the meantime.


-- 
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


[GitHub] [arrow-datafusion] mustafasrepo commented on pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
mustafasrepo commented on PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#issuecomment-1333468692

   > No, I think this change is not correct!!! WindowAggExec should always keep the input ordering, and the input ordering does not always equal with the required input ordering. For example, input ordering could be `order by ['a', 'b', 'c']`, the required input ordering is `order by ['a', 'b']`, `order by ['a', 'b', 'c']` can satisfy `order by ['a', 'b']`, in this case, the output ordering of the WindowAggExec itself is still `order by ['a', 'b', 'c']`.
   
   We have updated this PR to address this concern. Thanks for pointing it out @mingmwang .


-- 
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


[GitHub] [arrow-datafusion] ozankabak commented on pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
ozankabak commented on PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#issuecomment-1333727696

   Thanks! Let's conclude the discussion on your PR first, and then we can easily decide whether to go forward with this or not.


-- 
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


[GitHub] [arrow-datafusion] mustafasrepo closed pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
mustafasrepo closed pull request #4439: WindowAggExec output ordering is fixed
URL: https://github.com/apache/arrow-datafusion/pull/4439


-- 
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


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

Posted by 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


[GitHub] [arrow-datafusion] mustafasrepo commented on pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
mustafasrepo commented on PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#issuecomment-1333331141

   > #4332
   @mingmwang Thanks for your feedback. I saw this problem while working on a PR that removes unnecessary `SortExec` before `WindowAggExec` to improve efficiency. Consider the query below 
   
   ```sql
   SELECT
         c9,
         SUM(c9) OVER(ORDER BY c9) as sum1,
         SUM(c9) OVER(ORDER BY c9, c8) as sum2
         FROM aggregate_test_100
   ```
   
   Its physical plan is as follows 
   
   ```sql
   +---------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
   | plan_type     | plan                                                                                                                                                                                                                                                |
   +---------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
   | logical_plan  | Projection: aggregate_test_100.c9, SUM(aggregate_test_100.c9) ORDER BY [aggregate_test_100.c9 ASC NULLS LAST] AS sum1, SUM(aggregate_test_100.c9) ORDER BY [aggregate_test_100.c9 ASC NULLS LAST, aggregate_test_100.c8 ASC NULLS LAST] AS sum2     |
   |               |   WindowAggr: windowExpr=[[SUM(aggregate_test_100.c9) ORDER BY [aggregate_test_100.c9 ASC NULLS LAST]]]                                                                                                                                             |
   |               |     WindowAggr: windowExpr=[[SUM(aggregate_test_100.c9) ORDER BY [aggregate_test_100.c9 ASC NULLS LAST, aggregate_test_100.c8 ASC NULLS LAST]]]                                                                                                     |
   |               |       TableScan: aggregate_test_100 projection=[c8, c9]                                                                                                                                                                                             |
   | physical_plan | ProjectionExec: expr=[c9@3 as c9, SUM(aggregate_test_100.c9) ORDER BY [aggregate_test_100.c9 ASC NULLS LAST]@0 as sum1, SUM(aggregate_test_100.c9) ORDER BY [aggregate_test_100.c9 ASC NULLS LAST, aggregate_test_100.c8 ASC NULLS LAST]@1 as sum2] |
   |               |   WindowAggExec: wdw=[SUM(aggregate_test_100.c9): Ok(Field { name: "SUM(aggregate_test_100.c9)", data_type: UInt64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: None })]                                                          |
   |               |     SortExec: [c9@2 ASC NULLS LAST]                                                                                                                                                                                                                 |
   |               |       WindowAggExec: wdw=[SUM(aggregate_test_100.c9): Ok(Field { name: "SUM(aggregate_test_100.c9)", data_type: UInt64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: None })]                                                      |
   |               |         SortExec: [c9@1 ASC NULLS LAST,c8@0 ASC NULLS LAST]                                                                                                                                                                                         |
   |               |           CsvExec: files=[Users/akurmustafa/projects/synnada/arrow-datafusion-synnada/testing/data/csv/aggregate_test_100.csv], has_header=true, limit=None, projection=[c8, c9]                                                                    |
   |               |                                                                                                                                                                                                                                                     |
   +---------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
   ```
   
   Second (from the bottom) `SortExec` can be removed from the physical plan safely. To remove unnecessary `SortExec` from the physical plan, I compare the required sorting of `WindowAggExec` with its `input_exec.output_ordering()` at [here](https://github.com/apache/arrow-datafusion/blob/fdc83e8524df30ac5d0ae097572b7c48dc686ba9/datafusion/core/src/physical_plan/planner.rs#L518). Second `WindowAggExec` requires input_ordering `c9@2 ASC NULLS LAST`. Its input `WindowAggExec` should have `output_ordering` , `c9@1 ASC NULLS LAST,c8@0 ASC NULLS LAST`. However, `output_ordering` of the first `WindowAggExec` returns `None`. The reason is that since we did not put `SortExec` before it yet (It is is done at optimization pass), its input is `CsvExec` and its `ouput_ordering` is `None`. 
   In summary, unless we pass from `BasicEnforcement` optimization pass, the output ordering information of `WindowAggExec` would give output ordering of Executor before `SortExec` in the final physical plan.
   
   
   > @mustafasrepo Could you please explain more specifically why you can not rely on the current `output_ordering` result? Do you have your own physical optimizer rules?
   
   


-- 
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


[GitHub] [arrow-datafusion] mingmwang commented on pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
mingmwang commented on PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#issuecomment-1333570051

   @metesynnada 
   The rule `BasicEnforcement` is part of the `DefaultPhysicalPlanner`, that's why the rule is an enforcement rule(A common rule in many other DB Engines).
   


-- 
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


[GitHub] [arrow-datafusion] mingmwang commented on pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
mingmwang commented on PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#issuecomment-1332930260

   @mustafasrepo Could you please explain more specifically why you can not rely on the current `output_ordering` result?
   Do you have your own physical optimizer rules?
   


-- 
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


[GitHub] [arrow-datafusion] mustafasrepo commented on pull request #4439: WindowAggExec output ordering is fixed

Posted by GitBox <gi...@apache.org>.
mustafasrepo commented on PR #4439:
URL: https://github.com/apache/arrow-datafusion/pull/4439#issuecomment-1333344570

   > @mustafasrepo Could you please explain more specifically why you can not rely on the current `output_ordering` result? Do you have your own physical optimizer rules?
   
   To replicate my problem if we put the statement `println!("input exec output ordering:{:?}", input_exec.output_ordering()); `[here](https://github.com/apache/arrow-datafusion/blob/fdc83e8524df30ac5d0ae097572b7c48dc686ba9/datafusion/core/src/physical_plan/planner.rs#L518)
   For above query, we receive output ordering `input exec output ordering:None`, for both `WindowAggExec`s
   
   However, for the second  `WindowAggExec` we should receive `input exec output ordering:Some([PhysicalSortExpr { expr: Column { name: "c9", index: 1 }, options: SortOptions { descending: false, nulls_first: false } }, PhysicalSortExpr { expr: Column { name: "c8", index: 0 }, options: SortOptions { descending: false, nulls_first: false } }])`.


-- 
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