Message-ID: <6c67b7bb-5469-475f-8988-0a50b4c64852@postgrespro.ru>
Date: Tue, 26 Dec 2023 18:37:01 +0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: POC: GROUP BY optimization
Content-Language: en-US
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>,
 Tomas Vondra <tomas.vondra@enterprisedb.com>,
 Teodor Sigaev <teodor@sigaev.ru>, David Rowley <dgrowleyml@gmail.com>,
 "a.rybakina" <a.rybakina@postgrespro.ru>,
 =?UTF-8?B?0JHQtdC70Y/Qu9C+0LIg0JTQsNC80LjRgCDQndCw0LjQu9C10LLQuNGH?=
 <d.belyalov@postgrespro.ru>, Tom Lane <tgl@sss.pgh.pa.us>
References: <CA+q6zcVRrd-z4YZ4M43ccst7aGL9==w5r1fionRWhP9ot6mybQ@mail.gmail.com>
 <721b3ff9-f214-f5d4-86c7-bae515bbec18@enterprisedb.com>
 <7bea462e-8c3e-0f8c-c7d5-f4daa12f3baf@postgrespro.ru>
 <9af89543-3d17-4c98-59de-1daa5bceeb06@enterprisedb.com>
 <9408e450-60c6-6fbc-d5c4-467bb0abfe67@postgrespro.ru>
 <44472925-f6c6-e8de-6f72-0451458532c4@enterprisedb.com>
 <CALNJ-vQp3WNV-kKuGsjSOhjRfdECA1Dbxa-vjAgOmQ4f4Qaw-Q@mail.gmail.com>
 <7a80a207-5e8e-b80c-476c-2d290b0526e9@enterprisedb.com>
 <65941e87-86c1-c09c-8ad5-8e102aebe8ee@enterprisedb.com>
 <CAApHDvq65=9Ro+hLX1i9ugWEiNDvHrBibAO7ARcTnf38_JE+UQ@mail.gmail.com>
 <CAApHDvoEF0GsqmBuCPXb6f12jPwz8EeLG-oYsOSc5zrJtKZHOg@mail.gmail.com>
 <7256159a-bb83-7ef3-c9c9-b2188a29aaf8@postgrespro.ru>
 <60610df1-c32f-ebdf-e58c-7a664431f452@enterprisedb.com>
 <9acd27e9-3372-4d78-b9bc-73e407b8a007@postgrespro.ru>
 <dfd75679-088a-4905-9564-67c5f6421a19@postgrespro.ru>
 <CAPpHfdtzaVa7S4onKy3YvttF2rrH5hQNHx9HtcSTLbpjx+MJ+w@mail.gmail.com>
From: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
Organization: Postgres Professional
In-Reply-To: <CAPpHfdtzaVa7S4onKy3YvttF2rrH5hQNHx9HtcSTLbpjx+MJ+w@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/6c67b7bb-5469-475f-8988-0a50b4c64852%40postgrespro.ru>
Precedence: bulk

On 21/12/2023 17:53, Alexander Korotkov wrote:
> On Sun, Oct 1, 2023 at 11:45 AM Andrei Lepikhov
> <a.lepikhov@postgrespro.ru> wrote:
>> New version of the patch. Fixed minor inconsistencies and rebased onto
>> current master.
> Thank you (and other authors) for working on this subject.  Indeed to
> GROUP BY clauses are order-agnostic.  Reordering them in the most
> suitable order could give up significant query planning benefits.  I
> went through the thread: I see significant work has been already made
> on this patch, the code is quite polished.
Maybe, but issues, mentioned in [1], still not resolved. It is the only 
reason, why this thread hasn't been active.
> I'd like to make some notes.
> 1) As already mentioned, there is clearly a repetitive pattern for the
> code following after get_useful_group_keys_orderings() calls.  I think
> it would be good to extract it into a separate function.  Please, do
> this as a separate patch coming before the group-by patch. That would
> simplify the review.
Yeah, these parts of code a bit different. I will try to make common 
routine.
> 2) I wonder what planning overhead this patch could introduce?  Could
> you try to measure the worst case?  What if we have a table with a lot
> of indexes and a long list of group-by clauses partially patching
> every index.  This should give us an understanding on whether we need
> a separate GUC to control this feature.
Ok> 3) I see that get_useful_group_keys_orderings() makes 3 calls to
> get_cheapest_group_keys_order() function.  Each time
> get_cheapest_group_keys_order() performs the cost estimate and
> reorders the free keys.  However, cost estimation implies the system
> catalog lookups (that is quite expensive).  I wonder if we could
> change the algorithm.  Could we just sort the group-by keys by cost
> once, save this ordering and then just re-use it.  So, every time we
> need to reorder a group by, we can just pull the required keys to the
> top and use saved ordering for the rest.  I also wonder if we could do
> this once for add_paths_to_grouping_rel() and
> create_partial_grouping_paths() calls.  So, it probably should be
> somewhere in create_ordinary_grouping_paths().
Thanks for the idea!> 4) I think we can do some optimizations when 
enable_incremental_sort
> == off.  Then in get_useful_group_keys_orderings() we should only deal
> with input_path fully matching the group-by clause, and try only full
> match of group-by output to the required order.
Oh, we had designed before the incremental sort was invented. Will see 
what we can do here.

[1] 
https://www.postgresql.org/message-id/60610df1-c32f-ebdf-e58c-7a664431f452%40enterprisedb.com

-- 
regards,
Andrei Lepikhov
Postgres Professional