Horton

Re: Add proper planner support for ORDER BY / DISTINCT aggregates

David Rowley <dgrowleyml@gmail.com>

View thread View on postgr.es
From: David Rowley <dgrowleyml@gmail.com>
To: Ronan Dunklau <ronan.dunklau@aiven.io>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>, Ranier Vilela <ranier.vf@gmail.com>
Date: 2021-07-17T02:36:09Z
Lists: pgsql-hackers

Attachments

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

Commits

  1. Don't presort ORDER BY/DISTINCT Aggrefs with volatile functions

  2. Add enable_presorted_aggregate GUC

  3. Remove pessimistic cost penalization from Incremental Sort

  4. Fix hypothetical problem passing the wrong GROUP BY pathkeys

  5. Remove unused fields from ExprEvalStep

  6. Improve performance of ORDER BY / DISTINCT aggregates

  7. Refactor function parse_subscription_options.