public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ronan Dunklau <[email protected]>
To: David Rowley <[email protected]>
Cc: PostgreSQL Developers <[email protected]>
Cc: Ranier Vilela <[email protected]>
Subject: Re: Add proper planner support for ORDER BY / DISTINCT aggregates
Date: Wed, 21 Jul 2021 16:01:03 +0200
Message-ID: <5137746.gn36MdYUqo@aivenronan> (raw)
In-Reply-To: <CAApHDvogabtJzP8K6s75ruSXJAOfCvUJtUVWBS=2C-49kRqvCQ@mail.gmail.com>
References: <CAApHDvpHzfo92=R4W0+xVua3BUYCKMckWAmo-2t_KiXN-wYH=w@mail.gmail.com>
<4285468.TQqHX9kgJM@aivenronan>
<CAApHDvogabtJzP8K6s75ruSXJAOfCvUJtUVWBS=2C-49kRqvCQ@mail.gmail.com>
Le mercredi 21 juillet 2021, 04:52:43 CEST David Rowley a écrit :
> Thanks for finding this. I've made a few changes to make this case
> work as you describe. Please see attached v6 patches.
>
> Because I had to add additional code to take the GROUP BY pathkeys
> into account when choosing the best ORDER BY agg pathkeys, the
> function that does that became a little bigger. To try to reduce the
> complexity of it, I got rid of all the processing from the initial
> loop and instead it now uses the logic from the 2nd loop to find the
> best pathkeys. The reason I'd not done that in the first place was
> because I'd thought I could get away without building an additional
> Bitmapset for simple cases, but that's probably fairly cheap compared
> to building Pathkeys. With the additional complexity for the GROUP
> BY pathkeys, the extra code seemed not worth it.
>
> The 0001 patch is the ORDER BY aggregate code. 0002 is to fix up some
> broken regression tests in postgres_fdw that 0001 caused. It appears
> that 0001 uncovered a bug in the postgres_fdw code. I've reported
> that in [1]. If that turns out to be a bug then it'll need to be fixed
> before this can progress.
I tested the 0001 patch against both HEAD and my proposed bugfix for
postgres_fdw.
There is a problem that the ordered aggregate is not pushed down anymore. The
underlying Sort node is correctly pushed down though.
This comes from the fact that postgres_fdw grouping path never contains any
pathkey. Since the cost is fuzzily the same between the pushed-down aggregate
and the locally performed one, the tie is broken against the pathkeys.
Ideally we would add the group pathkeys to the grouping path, but this would
add an additional ORDER BY expression matching the GROUP BY. Moreover, some
triaging of the pathkeys would be necessary since we now mix the sort-in-
aggref pathkeys with the group_pathkeys.
--
Ronan Dunklau
view thread (64+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Add proper planner support for ORDER BY / DISTINCT aggregates
In-Reply-To: <5137746.gn36MdYUqo@aivenronan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox