On Fri, 30 Jun 2023 at 18:45, John Naylor <john.naylor@enterprisedb.com> wrote:
> Here is v3 with that change. I still need to make sure the tests cover all cases, so I'll do that as time permits.
Alsocreating CF entry.
Thanks for picking this back up again for the v17 cycle. I've reread
the entire thread to remind myself where we got to.
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 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.
David
[1] https://www.postgresql.org/message-id/attachment/143109/bench_windowsort.sh.txt
[2] https://www.postgresql.org/message-id/attachment/143112/orderby_windowclause_pushdown_testing_only.patch.txt
