wiedld commented on PR #14821: URL: https://github.com/apache/datafusion/pull/14821#issuecomment-2688837380
> but I don't agree the part where we filter the expressions being heterogeneously constant across partitions, while retaining the output ordering. > I cannot follow from which point this fix arose, but we should fix the constantness check instead, where this function is called and returns and unexpected result. I'm saying this because output_ordering() API reflects the per-partition behavior. @berkaysynnada -- once I merged in the latest main, a [test added by you](https://github.com/apache/datafusion/blob/f5b7affecd90e9be26289d869c4a542359cb98e3/datafusion/core/tests/physical_optimizer/enforce_sorting.rs#L2089) a few hours earlier started failing. Specifically, it was failing due to the removal of heterogeneous constant fields from the ordering requirements. Your test case is not failing on main since the SortExec is kept intact, instead of selectively recreated (if needed) from the ordering requirements. See here for exactly how this happens: https://github.com/apache/datafusion/pull/14821#issuecomment-2686306852 As a result, I proposed a possible "fix" (as mentioned here: https://github.com/apache/datafusion/pull/14821#issuecomment-2686475105) based upon my limited understanding of how constants (and if they are heterogeneous across partitions) should be handling when calculating the output ordering. But I'm not sure this is the proper fix. 🤔 I'm going to convert this PR back to a draft, and make a reproducer test case isolating how `output_ordering()` calculates based on heterogeneous constants. Then we can decide that behavior first (before re-considering this PR). -- 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
