On Mon, Jul 3, 2023 at 5:15 PM David Rowley <dgrowle...@gmail.com> wrote: > > On Fri, 30 Jun 2023 at 18:45, John Naylor <john.nay...@enterprisedb.com> wrote:
> I looked over your patch and don't see anything to report aside from > the unfinished/undecided part around the tiebreak function for > tuplesort_begin_index_hash(). I went ahead and added a degenerate function, just for consistency -- might also head off possible alarms from code analysis tools. > I also ran the benchmark script [1] with the patch from [2] and > calculated the speedup with [2] with and without your v3 patch. I've > attached two graphs with the benchmark results. Any value >100% > indicates that performing the sort for the ORDER BY at the same time > as the WindowAgg improves performance, whereas anything < 100% > indicates a regression. The bars in blue show the results without > your v3 patch and the red bars show the results with your v3 patch. > Looking at the remaining regressions it does not really feel like > we've found the culprit for the regressions. Certainly, v3 helps, but > I just don't think it's to the level we'd need to make the window sort > changes a good idea. > > I'm not sure exactly how best to proceed here. I think the tiebreak > stuff is worth doing regardless, so maybe that can just go in to > eliminate that as a factor and we or I can continue to see what else > is to blame. Thanks for testing again. Sounds good, I removed a now-invalidated comment, pgindent'd, and pushed. -- John Naylor EDB: http://www.enterprisedb.com