public inbox for [email protected]  
help / color / mirror / Atom feed
From: Andrei Lepikhov <[email protected]>
To: Alexander Korotkov <[email protected]>
Cc: Pavel Borisov <[email protected]>
Cc: vignesh C <[email protected]>
Cc: PostgreSQL Developers <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Teodor Sigaev <[email protected]>
Cc: David Rowley <[email protected]>
Cc: a.rybakina <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: Re: POC: GROUP BY optimization
Date: Sun, 14 Jan 2024 19:14:13 +0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAPpHfdscoK4u4PhR354Swso3Tx0cFOjJKcbG02mi98jTwRxW9A@mail.gmail.com>
References: <CA+q6zcVRrd-z4YZ4M43ccst7aGL9==w5r1fionRWhP9ot6mybQ@mail.gmail.com>
	<CAApHDvq65=9Ro+hLX1i9ugWEiNDvHrBibAO7ARcTnf38_JE+UQ@mail.gmail.com>
	<CAApHDvoEF0GsqmBuCPXb6f12jPwz8EeLG-oYsOSc5zrJtKZHOg@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAPpHfdtzaVa7S4onKy3YvttF2rrH5hQNHx9HtcSTLbpjx+MJ+w@mail.gmail.com>
	<[email protected]>
	<CALDaNm2CP2XQGTdNYKHnETeQZUV_TUBM-S297-298VqiwrFrLg@mail.gmail.com>
	<[email protected]>
	<CALT9ZEFbBHxjaFwRxhqf1tj2M5wmGiiZwUY5ts8eq3jwxFJ_mg@mail.gmail.com>
	<CAPpHfdsrEfOqJfNuObNvM+-3D4sVzsCkmJXLLNZKf8t04dUgsg@mail.gmail.com>
	<[email protected]>
	<CAPpHfdscoK4u4PhR354Swso3Tx0cFOjJKcbG02mi98jTwRxW9A@mail.gmail.com>

On 13/1/2024 22:00, Alexander Korotkov wrote:
> On Sat, Jan 13, 2024 at 11:09 AM Andrei Lepikhov
> <[email protected]> wrote:
>> On 11/1/2024 18:30, Alexander Korotkov wrote:
>>> On Tue, Jan 9, 2024 at 1:14 PM Pavel Borisov <[email protected]> wrote:
>>>>> Hmm, I don't see this old code in these patches. Resend 0002-* because
>>>>> of trailing spaces.
>>>>
>>>>
>>>> AFAIK, cfbot does not seek old versions of patchset parts in previous messages. So for it to run correctly, a new version of the whole patchset should be sent even if most patches are unchanged.
>>>
>>> Please, find the revised patchset with some refactoring and comments
>>> improvement from me.  I'll continue looking into this.
>> The patch looks better, thanks to your refactoring.
>> I propose additional comments and tests to make the code more
>> understandable (see attachment).
>> I intended to look into this part of the code more, but the tests show a
>> difference between PostgreSQL v.15 and v.16, which causes changes in the
>> code of this feature.
> 
> Makes sense.  I've incorporated your changes into the patchset.
One more improvement. To underpin code change:

-               return cur_ec;  /* Match! */
+           {
+               /*
+                * Match!
+                *
+                * Copy the sortref if it wasn't set yet. That may happen if
+                * the ec was constructed from WHERE clause, i.e. it doesn't
+                * have a target reference at all.
+                */
+               if (cur_ec->ec_sortref == 0 && sortref > 0)
+                   cur_ec->ec_sortref = sortref;
+               return cur_ec;
+           }

I propose the test (see attachment). It shows why we introduce this 
change: GROUP-BY should juggle not only pathkeys generated by explicit 
sort operators but also planner-made, likewise in this example, by 
MergeJoin.

-- 
regards,
Andrei Lepikhov
Postgres Professional

diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index 0d46e096e5..ca38e78f21 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -2879,6 +2879,37 @@ FROM t1 WHERE c2 < 100 GROUP BY c1 ORDER BY 2;
 (10 rows)
 
 DROP TABLE t1 CASCADE;
+-- Check, that GROUP-BY reordering optimization can operate with pathkeys, built
+-- by planner itself. For example, by MergeJoin.
+SET enable_hashjoin = off;
+SET enable_nestloop = off;
+explain (COSTS OFF)
+SELECT c1.relname,c1.relpages
+FROM pg_class c1 JOIN pg_class c2 ON (c1.relname=c2.relname AND c1.relpages=c2.relpages)
+GROUP BY c1.reltuples,c1.relpages,c1.relname
+ORDER BY c1.relpages, c1.relname, c1.relpages*c1.relpages;
+                                         QUERY PLAN                                          
+---------------------------------------------------------------------------------------------
+ Incremental Sort
+   Sort Key: c1.relpages, c1.relname, ((c1.relpages * c1.relpages))
+   Presorted Key: c1.relpages, c1.relname
+   ->  Group
+         Group Key: c1.relpages, c1.relname, c1.reltuples
+         ->  Incremental Sort
+               Sort Key: c1.relpages, c1.relname, c1.reltuples
+               Presorted Key: c1.relpages, c1.relname
+               ->  Merge Join
+                     Merge Cond: ((c1.relpages = c2.relpages) AND (c1.relname = c2.relname))
+                     ->  Sort
+                           Sort Key: c1.relpages, c1.relname
+                           ->  Seq Scan on pg_class c1
+                     ->  Sort
+                           Sort Key: c2.relpages, c2.relname
+                           ->  Seq Scan on pg_class c2
+(16 rows)
+
+RESET enable_hashjoin;
+RESET enable_nestloop;
 RESET enable_hashagg;
 RESET max_parallel_workers;
 RESET max_parallel_workers_per_gather;
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index f99167ac9e..cf87b5d5dd 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -1233,6 +1233,18 @@ SELECT array_agg(c1 ORDER BY c2),c2
 FROM t1 WHERE c2 < 100 GROUP BY c1 ORDER BY 2;
 DROP TABLE t1 CASCADE;
 
+-- Check, that GROUP-BY reordering optimization can operate with pathkeys, built
+-- by planner itself. For example, by MergeJoin.
+SET enable_hashjoin = off;
+SET enable_nestloop = off;
+explain (COSTS OFF)
+SELECT c1.relname,c1.relpages
+FROM pg_class c1 JOIN pg_class c2 ON (c1.relname=c2.relname AND c1.relpages=c2.relpages)
+GROUP BY c1.reltuples,c1.relpages,c1.relname
+ORDER BY c1.relpages, c1.relname, c1.relpages*c1.relpages;
+RESET enable_hashjoin;
+RESET enable_nestloop;
+
 RESET enable_hashagg;
 RESET max_parallel_workers;
 RESET max_parallel_workers_per_gather;


Attachments:

  [text/plain] details-2.txt (2.7K, ../[email protected]/2-details-2.txt)
  download | inline diff:
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index 0d46e096e5..ca38e78f21 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -2879,6 +2879,37 @@ FROM t1 WHERE c2 < 100 GROUP BY c1 ORDER BY 2;
 (10 rows)
 
 DROP TABLE t1 CASCADE;
+-- Check, that GROUP-BY reordering optimization can operate with pathkeys, built
+-- by planner itself. For example, by MergeJoin.
+SET enable_hashjoin = off;
+SET enable_nestloop = off;
+explain (COSTS OFF)
+SELECT c1.relname,c1.relpages
+FROM pg_class c1 JOIN pg_class c2 ON (c1.relname=c2.relname AND c1.relpages=c2.relpages)
+GROUP BY c1.reltuples,c1.relpages,c1.relname
+ORDER BY c1.relpages, c1.relname, c1.relpages*c1.relpages;
+                                         QUERY PLAN                                          
+---------------------------------------------------------------------------------------------
+ Incremental Sort
+   Sort Key: c1.relpages, c1.relname, ((c1.relpages * c1.relpages))
+   Presorted Key: c1.relpages, c1.relname
+   ->  Group
+         Group Key: c1.relpages, c1.relname, c1.reltuples
+         ->  Incremental Sort
+               Sort Key: c1.relpages, c1.relname, c1.reltuples
+               Presorted Key: c1.relpages, c1.relname
+               ->  Merge Join
+                     Merge Cond: ((c1.relpages = c2.relpages) AND (c1.relname = c2.relname))
+                     ->  Sort
+                           Sort Key: c1.relpages, c1.relname
+                           ->  Seq Scan on pg_class c1
+                     ->  Sort
+                           Sort Key: c2.relpages, c2.relname
+                           ->  Seq Scan on pg_class c2
+(16 rows)
+
+RESET enable_hashjoin;
+RESET enable_nestloop;
 RESET enable_hashagg;
 RESET max_parallel_workers;
 RESET max_parallel_workers_per_gather;
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index f99167ac9e..cf87b5d5dd 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -1233,6 +1233,18 @@ SELECT array_agg(c1 ORDER BY c2),c2
 FROM t1 WHERE c2 < 100 GROUP BY c1 ORDER BY 2;
 DROP TABLE t1 CASCADE;
 
+-- Check, that GROUP-BY reordering optimization can operate with pathkeys, built
+-- by planner itself. For example, by MergeJoin.
+SET enable_hashjoin = off;
+SET enable_nestloop = off;
+explain (COSTS OFF)
+SELECT c1.relname,c1.relpages
+FROM pg_class c1 JOIN pg_class c2 ON (c1.relname=c2.relname AND c1.relpages=c2.relpages)
+GROUP BY c1.reltuples,c1.relpages,c1.relname
+ORDER BY c1.relpages, c1.relname, c1.relpages*c1.relpages;
+RESET enable_hashjoin;
+RESET enable_nestloop;
+
 RESET enable_hashagg;
 RESET max_parallel_workers;
 RESET max_parallel_workers_per_gather;


view thread (136+ 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], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: POC: GROUP BY optimization
  In-Reply-To: <[email protected]>

* 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