On Fri, 16 Jul 2021 at 22:00, David Rowley <dgrowleyml@gmail.com> wrote:
>
> 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.
Looks like I did a sloppy job of that. I messed up the condition in
standard_qp_callback() that sets the ORDER BY aggregate pathkeys so
that it accidentally set them when there was an unsortable GROUP BY
clause, as highlighted by the postgres_fdw tests failing. I've now
added a comment to explain why the condition is the way it is so that
I don't forget again.
Here's a cleaned-up version that passes make check-world.
David