cool9850311 commented on PR #26773: URL: https://github.com/apache/datafusion/pull/26773#issuecomment-6570687935
> Optional cleanup: the PR description has fallen behind the commits. It still says four slt cases (there are seven), says no existing expectation changed (five `window.slt` EXPLAINs did), and says results were always correct, though the stacked-window case was wrong on `main`. The closing note also still lists `Absolute` as unfixed, and [0406f60](https://github.com/apache/datafusion/commit/0406f60a0554bf783437b30bb4c5d8c5094f38fc) isn't mentioned. Could you refresh it, since it becomes the squash commit message? Refreshed. One correction first: my earlier comment said the description had been rewritten, but it hadn't -- the rewrite only ever existed in my working copy, so all five of those were still on the page. It now has nine slt cases rather than four and what each holds down, the five window.slt fetches that do change with why, `LimitEffect::Absolute` as fixed, and the `InputOrderMode` commit the old body never mentioned. "Results were always correct" is reworded rather than dropped: the missed optimization in the issue really didn't change results, but two of the fixes here are wrong-results bugs and one of those is wrong on main too, so it now says which is which. e93950b applies the same argument to the tests: three comments said "from review", "before this PR" and "the first commit in this PR", which point at nothing once this squashes. Also rebased onto current main -- no conflicts, every commit kept its patch-id. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
