On Fri, 16 Jul 2021 at 18:04, David Rowley <dgrowleyml@gmail.com> wrote:
> What we maybe could consider instead would be to pick the first Aggref
> then look for the most sorted derivative of that then tally up the
> number of Aggrefs that can be sorted using those pathkeys, then repeat
> that process for the remaining Aggrefs that didn't have the same
> prefix then use the pathkeys for the set with the most Aggrefs. We
> could still tiebreak on the targetlist position so at least it's not
> random which ones we pick. Now that we have a list of Aggrefs that are
> deduplicated in the planner thanks to 0a2bc5d61e it should be fairly
> easy to do that.
I've attached a patch which does as I mention above.
I'm still not sold on if this is better than just going with the order
of the first aggregate. The problem might be that a plan could change
as new aggregates are added to the end of the target list. It feels
like there might be a bit less control over it than the previous
version. Remember that suiting more aggregates is not always better as
there might be an index that could provide presorted input for another
set of aggregates which would overall reduce the number of sorts.
However, maybe it's not too big an issue as for any aggregates that
are not presorted we're left doing 1 sort per Aggref, so reducing the
number of those might be more important than selecting the order that
has an index to support it.
I've left off the 0002 patch this time as I think the lack of JIT
support for DISTINCT was causing the CF bot to fail. I'd quite like
to confirm that theory.
David