Content-Type: multipart/mixed; boundary="------------PHLSBMgPC0w0H0LgJhfbzOV7"
Message-ID: <e225591f-50bf-4b35-8b87-8107477804d2@postgrespro.ru>
Date: Sun, 4 Feb 2024 11:57:11 +0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: v17 Possible Union All Bug
From: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
To: Alexander Korotkov <aekorotkov@gmail.com>,
 "David G. Johnston" <david.g.johnston@gmail.com>,
 Tom Lane <tgl@sss.pgh.pa.us>
Cc: David Rowley <dgrowleyml@gmail.com>,
 Alexander Korotkov <akorotkov@postgresql.org>,
 PostgreSQL Bug List <pgsql-bugs@lists.postgresql.org>,
 Richard Guo <guofenglinux@gmail.com>
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>
Content-Language: en-US
Organization: Postgres Professional
In-Reply-To: <a9440b72-91c3-4041-89ec-531de5c2a5bc@postgrespro.ru>
Archived-At: 
 <https://www.postgresql.org/message-id/e225591f-50bf-4b35-8b87-8107477804d2%40postgrespro.ru>
Precedence: bulk

This is a multi-part message in MIME format.
--------------PHLSBMgPC0w0H0LgJhfbzOV7
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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

--------------PHLSBMgPC0w0H0LgJhfbzOV7
Content-Type: text/plain; charset=UTF-8; name="naive_fix.diff"
Content-Disposition: attachment; filename="naive_fix.diff"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL29wdGltaXplci9wYXRoL3BhdGhrZXlzLmMgYi9z
cmMvYmFja2VuZC9vcHRpbWl6ZXIvcGF0aC9wYXRoa2V5cy5jCmluZGV4IDgyZmYzMTI3M2Iu
LmY4OTc0NWYwNjMgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL29wdGltaXplci9wYXRoL3Bh
dGhrZXlzLmMKKysrIGIvc3JjL2JhY2tlbmQvb3B0aW1pemVyL3BhdGgvcGF0aGtleXMuYwpA
QCAtMzY5LDYgKzM2OSw3IEBAIGdyb3VwX2tleXNfcmVvcmRlcl9ieV9wYXRoa2V5cyhMaXN0
ICpwYXRoa2V5cywgTGlzdCAqKmdyb3VwX3BhdGhrZXlzLAogewogCUxpc3QJICAgKm5ld19n
cm91cF9wYXRoa2V5cyA9IE5JTCwKIAkJCSAgICpuZXdfZ3JvdXBfY2xhdXNlcyA9IE5JTDsK
KwlMaXN0CSAgICpncm91cGluZ19wYXRoa2V5cyA9IGxpc3RfY29weV9oZWFkKCpncm91cF9w
YXRoa2V5cywgbnVtX2dyb3VwYnlfcGF0aGtleXMpOwogCUxpc3RDZWxsICAgKmxjOwogCWlu
dAkJCW47CiAKQEAgLTM5NSw3ICszOTYsNyBAQCBncm91cF9rZXlzX3Jlb3JkZXJfYnlfcGF0
aGtleXMoTGlzdCAqcGF0aGtleXMsIExpc3QgKipncm91cF9wYXRoa2V5cywKIAkJICogdGhl
cmUgaXMgbm8gc29ydGNsYXVzZSByZWZlcmVuY2UgZm9yIHNvbWUgcmVhc29uLgogCQkgKi8K
IAkJaWYgKGZvcmVhY2hfY3VycmVudF9pbmRleChsYykgPj0gbnVtX2dyb3VwYnlfcGF0aGtl
eXMgfHwKLQkJCSFsaXN0X21lbWJlcl9wdHIoKmdyb3VwX3BhdGhrZXlzLCBwYXRoa2V5KSB8
fAorCQkJIWxpc3RfbWVtYmVyX3B0cihncm91cGluZ19wYXRoa2V5cywgcGF0aGtleSkgfHwK
IAkJCXBhdGhrZXktPnBrX2VjbGFzcy0+ZWNfc29ydHJlZiA9PSAwKQogCQkJYnJlYWs7CiAK
QEAgLTQyOSw2ICs0MzAsNyBAQCBncm91cF9rZXlzX3Jlb3JkZXJfYnlfcGF0aGtleXMoTGlz
dCAqcGF0aGtleXMsIExpc3QgKipncm91cF9wYXRoa2V5cywKIAkqZ3JvdXBfY2xhdXNlcyA9
IGxpc3RfY29uY2F0X3VuaXF1ZV9wdHIobmV3X2dyb3VwX2NsYXVzZXMsCiAJCQkJCQkJCQkJ
CSpncm91cF9jbGF1c2VzKTsKIAorCXBmcmVlKGdyb3VwaW5nX3BhdGhrZXlzKTsKIAlyZXR1
cm4gbjsKIH0KIAo=

--------------PHLSBMgPC0w0H0LgJhfbzOV7--