wiedld commented on PR #14821: URL: https://github.com/apache/datafusion/pull/14821#issuecomment-2686306852
> There is a new test on main ([added 4 hours ago](https://github.com/apache/datafusion/blob/f5b7affecd90e9be26289d869c4a542359cb98e3/datafusion/core/tests/physical_optimizer/enforce_sorting.rs#L2089)) which is failing. Converting to draft while I debug. It's failing sort pushdown on this test case: ``` "SortExec: expr=[non_nullable_col@1 ASC NULLS LAST, count@2 ASC NULLS LAST], preserve_partitioning=[false]", " WindowAggExec: wdw=[count: Ok(Field { name: \"count\", data_type: Int64, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} }), frame: WindowFrame { units: Rows, start_bound: Preceding(UInt64(NULL)), end_bound: Following(UInt64(NULL)), is_causal: false }]", " DataSourceExec: file_groups={1 group: [[x]]}, projection=[nullable_col, non_nullable_col], output_ordering=[nullable_col@0 ASC NULLS LAST], file_type=parquet", ``` It tries to push down only the `non_nullable_col@1 ASC NULLS LAST` as the [current_plan_reqs](https://github.com/apache/datafusion/blob/238cb46cffd150966153dfd5e6dfbd9a5024a60e/datafusion/physical-optimizer/src/enforce_sorting/sort_pushdown.rs#L104-L107), and it removes the second `count` column since it's a constant (per when the [SortExec calculates its properties here).](https://github.com/apache/datafusion/blob/f5b7affecd90e9be26289d869c4a542359cb98e3/datafusion/physical-plan/src/sorts/sort.rs#L973) The reason this doesn't happen on main is because they (1) [immediately see if they can push down](https://github.com/apache/datafusion/blob/f5b7affecd90e9be26289d869c4a542359cb98e3/datafusion/physical-optimizer/src/enforce_sorting/sort_pushdown.rs#L125) the sort to it's child (without the recursive call to walk down to the child), and then (2) if it cannot be pushed down then re-use the same SortExec node ([this branch here](https://github.com/apache/datafusion/blob/f5b7affecd90e9be26289d869c4a542359cb98e3/datafusion/physical-optimizer/src/enforce_sorting/sort_pushdown.rs#L145-L148)), rather than reconstructing from the equivalence properties' ordering (which has constants removed). -- 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
