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: Justin Pryzby <pryzby@telsasoft.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Richard Guo <guofenglinux@gmail.com>, Ronan Dunklau <ronan.dunklau@aiven.io>, pgsql-hackers@lists.postgresql.org, Ranier Vilela <ranier.vf@gmail.com>
Date: 2022-08-17T23:38:25Z
Lists: pgsql-hackers 
On Wed, 17 Aug 2022 at 13:57, Justin Pryzby <pryzby@telsasoft.com> wrote:
> But in a few places, it removes the locally-computed group_pathkeys:
>
> -                       List       *group_pathkeys = root->group_pathkeys;

> I noticed because that creates a new shadow variable, which seems accidental.

Thanks for the report.

I've just pushed a fix for this that basically just removes the line
you quoted.  Really I should have been using the version of
group_pathkeys that stripped off the pathkeys from the ORDER BY /
DISTINCT aggregates that is calculated earlier in that function.   In
practice, there was no actual bug here as the wrong variable was only
being used in the code path that was handling partial paths.  We never
create any partial paths when there are aggregates with ORDER BY /
DISTINCT  clauses, so in that code path, the two versions of the
group_pathkeys variable would have always been set to the same thing.

It makes sense just to get rid of the shadowed variable since the
value of it will be the same anyway.

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.