From: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
To: Alexander Korotkov <aekorotkov@gmail.com>
To: David G. Johnston <david.g.johnston@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: David Rowley <dgrowleyml@gmail.com>
Cc: Alexander Korotkov <akorotkov@postgresql.org>
Cc: PostgreSQL Bug List <pgsql-bugs@lists.postgresql.org>
Cc: Richard Guo <guofenglinux@gmail.com>
Subject: Re: v17 Possible Union All Bug
Date: Sun, 4 Feb 2024 11:57:11 +0700
Message-ID: <e225591f-50bf-4b35-8b87-8107477804d2@postgrespro.ru> (raw)
In-Reply-To: <a9440b72-91c3-4041-89ec-531de5c2a5bc@postgrespro.ru>
References: <CAKFQuwY3Ek=cLThgd8FdaSc5JRDVt0FaV00gMcWra+TAR4gGUw@mail.gmail.com>
<CAKFQuwZKMbccEbjfLmpaBmD+heXRa-Cmf+XOKRe=mOxc9HYpqQ@mail.gmail.com>
<CAApHDvoJ21egQX=Mf015PBVY_9AoPhrdw-3Xpxr+Re_UjvnFTQ@mail.gmail.com>
<CAApHDvqGS=y2EgzLLJLBqPu0Cms9kecVs7H3ZG2pz9ofL9r3zg@mail.gmail.com>
<CAKFQuwaahSXUgT1Vpzm_L8YY3g=j8Hc0u0UTeXkNHHyff77Bcw@mail.gmail.com>
<CAKFQuwbp8cFu_w88cD8M9s6YppnBL1KkJOC9ht8E7MLscUTqvQ@mail.gmail.com>
<CAPpHfds_6Bk2H2FE_AxPTQ7A4cTWXCFVyXaX2TA4t5BUb-q46A@mail.gmail.com>
<41499cc3-31a1-4c9e-85d6-45e6d901ea6a@postgrespro.ru>
<c566aa61-b381-4420-ade9-24d0a01b71be@postgrespro.ru>
<a9440b72-91c3-4041-89ec-531de5c2a5bc@postgrespro.ru>
And finally, I've got the synthetic test:
CREATE TABLE mess_grouping (x integer, y integer, z integer, w integer,
f integer);
INSERT INTO mess_grouping (x,y,z,w,f) (SELECT x%10, x % 2, x%2, 2, x%10
FROM generate_series(1,100) AS x);
ANALYZE mess_grouping;
SET enable_nestloop = 'off';
SET enable_hashjoin = 'off';
SET enable_hashagg = 'off';
SET enable_group_by_reordering = 'on';
SELECT c1.z, c1.w, string_agg(''::text, repeat(''::text, c1.f) ORDER BY
c1.x,c1.y)
FROM mess_grouping c1 JOIN mess_grouping c2 ON (c1.x = c2.f)
GROUP BY c1.w, c1.z;
SET enable_group_by_reordering = 'off';
SELECT c1.z, c1.w, string_agg(''::text, repeat(''::text, c1.f) ORDER BY
c1.x,c1.y)
FROM mess_grouping c1 JOIN mess_grouping c2 ON (c1.x = c2.f)
GROUP BY c1.w, c1.z;
DROP TABLE IF EXISTS mess_grouping CASCADE;
You can see here, that first query execution produces:
z | w | string_agg
---+---+------------
0 | 2 |
1 | 2 |
0 | 2 |
1 | 2 |
0 | 2 |
1 | 2 |
0 | 2 |
1 | 2 |
0 | 2 |
1 | 2 |
(10 rows)
and second execution gives correct result:
z | w | string_agg
---+---+------------
0 | 2 |
1 | 2 |
(2 rows)
The simple fix is in the attachment. But I'm not sure we should fix
GROUP-BY optimization instead of the more general issue.
The source of the problem is root->group_pathkeys, which contains
grouping pathkeys and aggregate pathkeys. For now, their 'sortref'
values could intersect, and we can differ which one references the query
target list and which one the target list of the aggregate.
So, I would like to get advice here: should we make a quick fix here, or
is such a mess in the sortref values not a mess and designed for some
purposes?
--
regards,
Andrei Lepikhov
Postgres Professional
Attachments:naive_fix.diff (text/plain, 1.1K) downloaddiff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.cindex 82ff31273b..f89745f063 100644--- a/src/backend/optimizer/path/pathkeys.c+++ b/src/backend/optimizer/path/pathkeys.c@@ -369,6 +369,7 @@ group_keys_reorder_by_pathkeys(List *pathkeys, List **group_pathkeys,
{
List *new_group_pathkeys = NIL,
*new_group_clauses = NIL;
+ List *grouping_pathkeys = list_copy_head(*group_pathkeys, num_groupby_pathkeys);
ListCell *lc;
int n;
@@ -395,7 +396,7 @@ group_keys_reorder_by_pathkeys(List *pathkeys, List **group_pathkeys,
* there is no sortclause reference for some reason.
*/
if (foreach_current_index(lc) >= num_groupby_pathkeys ||
- !list_member_ptr(*group_pathkeys, pathkey) ||+ !list_member_ptr(grouping_pathkeys, pathkey) ||
pathkey->pk_eclass->ec_sortref == 0)
break;
@@ -429,6 +430,7 @@ group_keys_reorder_by_pathkeys(List *pathkeys, List **group_pathkeys,
*group_clauses = list_concat_unique_ptr(new_group_clauses,
*group_clauses);
+ pfree(grouping_pathkeys);
return n;
}
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: pgsql-bugs@postgresql.org
Cc: a.lepikhov@postgrespro.ru, aekorotkov@gmail.com, david.g.johnston@gmail.com, tgl@sss.pgh.pa.us, dgrowleyml@gmail.com, akorotkov@postgresql.org, pgsql-bugs@lists.postgresql.org, guofenglinux@gmail.com
Subject: Re: v17 Possible Union All Bug
In-Reply-To: <e225591f-50bf-4b35-8b87-8107477804d2@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox