public inbox for [email protected]  
help / color / mirror / Atom feed
From: Richard Guo <[email protected]>
To: David Rowley <[email protected]>
Cc: Ronan Dunklau <[email protected]>
Cc: PostgreSQL Developers <[email protected]>
Cc: Ranier Vilela <[email protected]>
Subject: Re: Add proper planner support for ORDER BY / DISTINCT aggregates
Date: Fri, 22 Jul 2022 17:33:24 +0800
Message-ID: <CAMbWs49hcB-Mj3MJTOesL5=y4oU+md=zEvDYdCBrOSaDvXLRGQ@mail.gmail.com> (raw)
In-Reply-To: <CAApHDvqS-p57j++gLMyQXmttk_MM3u243dWQ54FyW-7=4GP+1Q@mail.gmail.com>
References: <CAApHDvpHzfo92=R4W0+xVua3BUYCKMckWAmo-2t_KiXN-wYH=w@mail.gmail.com>
	<CAApHDvrXnUFNiYQiHv4Eu_rtbMPsKeVT-KG93_dD-ZYs9G5OnA@mail.gmail.com>
	<4493999.1py09z8qHF@aivenronan>
	<3163474.aeNJFYEL58@aivenronan>
	<CAApHDvqS-p57j++gLMyQXmttk_MM3u243dWQ54FyW-7=4GP+1Q@mail.gmail.com>

On Wed, Jul 20, 2022 at 1:27 PM David Rowley <[email protected]> wrote:

> I've been working on this patch again. There was a bit of work to do
> to rebase it atop db0d67db2.  The problem there was that since this
> patch appends pathkeys to suit ORDER BY / DISTINCT aggregates to the
> query's group_pathkeys, db0d67db2 goes and tries to rearrange those,
> but fails to find the SortGroupClause corresponding to the PathKey in
> group_pathkeys. I wish the code I came up with to make that work was a
> bit nicer, but what's there at least seems to work. There are a few
> more making copies of Lists than I'd like.


We may need to do more checks when adding members to 'aggindexes' to
record we've found pathkeys for an aggregate, because 'currpathkeys' may
include pathkeys for some latter aggregates. I can see this problem with
the query below:

    select max(b order by b), max(a order by a) from t group by a;

When processing the first aggregate, we compose the 'currpathkeys' as
{a, b} and mark this aggregate in 'aggindexes'. When it comes to the
second aggregate, we compose its pathkeys as {a} and decide that it is
not stronger than 'currpathkeys'. So the second aggregate is not
recorded in 'aggindexes'. As a result, we fail to mark aggpresorted for
the second aggregate.

Thanks
Richard


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], [email protected]
  Subject: Re: Add proper planner support for ORDER BY / DISTINCT aggregates
  In-Reply-To: <CAMbWs49hcB-Mj3MJTOesL5=y4oU+md=zEvDYdCBrOSaDvXLRGQ@mail.gmail.com>

* 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