MIME-Version: 1.0
References: <ZQhYYRhUxpW3PSf9@telsasoft.com> <CAMbWs48X+cK2pEaZDVVjd+nOJzb1dN1CO1OKz+6D0FX2OTCGbg@mail.gmail.com>
 <CAApHDvpJJigQRW29TppTOPYp+Aui0mtd3MpfRxyKv=N-tB62jQ@mail.gmail.com>
 <CAApHDvo7RzcQYw-gnkZr6QCijCqf8vJLkJ4XFk-KawvyAw109Q@mail.gmail.com>
 <CAApHDvqoMf_kxQKON+FLQh8DbMKe62R=OdrZoenb6xDYWmQVUQ@mail.gmail.com>
 <CAMbWs48VKgJyqfDNtKsYnNsPYWqA=stPKenehVz1NN0HTFV9mg@mail.gmail.com>
 <CAApHDvrAHkghCDNFG5+K2adWCDCC9cdZT2KZamVRkqYy31kL9w@mail.gmail.com>
 <CAApHDvqira92LLPbW37azL+fDEm7yvWni89YekB9G751W7nEBw@mail.gmail.com> <cf63174c-8c89-3953-cb49-48f41f74941a@gmail.com>
In-Reply-To: <cf63174c-8c89-3953-cb49-48f41f74941a@gmail.com>
From: David Rowley <dgrowleyml@gmail.com>
Date: Thu, 14 Mar 2024 12:00:24 +1300
Message-ID: <CAApHDvqEdHSMwZt5ngXOBK+rZ=WU5iJdkvf+yD7qQJVu606fNg@mail.gmail.com>
Subject: Re: pg16: XX000: could not find pathkey item to sort
To: Alexander Lakhin <exclusion@gmail.com>, Robert Haas <robertmhaas@gmail.com>, 
	Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Cc: Richard Guo <guofenglinux@gmail.com>, Justin Pryzby <pryzby@telsasoft.com>, 
	pgsql-hackers@lists.postgresql.org
Content-Type: multipart/mixed; boundary="0000000000002ed10b061392be13"
Archived-At: <https://www.postgresql.org/message-id/CAApHDvqEdHSMwZt5ngXOBK%2BrZ%3DWU5iJdkvf%2ByD7qQJVu606fNg%40mail.gmail.com>
Precedence: bulk

--0000000000002ed10b061392be13
Content-Type: text/plain; charset="UTF-8"

On Thu, 14 Mar 2024 at 06:00, Alexander Lakhin <exclusion@gmail.com> wrote:
> I've stumbled upon the same error, but this time it apparently has another
> cause. It can be produced (on REL_16_STABLE and master) as follows:
> CREATE TABLE t (a int, b int) PARTITION BY RANGE (a);
> CREATE TABLE td PARTITION OF t DEFAULT;
> CREATE TABLE tp1 PARTITION OF t FOR VALUES FROM (1) TO (2);
> SET enable_partitionwise_aggregate = on;
> SET parallel_setup_cost = 0;
> SELECT a, sum(b order by b) FROM t GROUP BY a ORDER BY a;
>
> ERROR:  could not find pathkey item to sort
>
> `git bisect` for this anomaly blames the same commit 1349d2790.

Thanks for finding and for the recreator script.

I've attached a patch which fixes the problem for me.

On debugging this I uncovered some other stuff that looks broken which
seems to caused by partition-wise aggregates.  With your example
query, in get_useful_pathkeys_for_relation(), we call
relation_can_be_sorted_early() to check if the pathkey can be used as
a set of pathkeys in useful_pathkeys_list.  The problem is that in
your query the 'rel' is the base relation belonging to the partitioned
table and relation_can_be_sorted_early() looks through the targetlist
for that relation and finds columns "a" and "b" in there.  The problem
is "b" has been aggregated away as partial aggregation has taken place
due to the partition-wise aggregation. I believe whichever rel we
should be using there should have an Aggref in the target exprs rather
than the plain unaggregated column.  I've added Robert and Ashutosh to
see what their thoughts are on this.

David

--0000000000002ed10b061392be13
Content-Type: text/plain; charset="US-ASCII"; name="fix_groupby_pathkeys.patch"
Content-Disposition: attachment; filename="fix_groupby_pathkeys.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_ltqe6xtv0>
X-Attachment-Id: f_ltqe6xtv0

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL29wdGltaXplci9wbGFuL3BsYW5uZXIuYyBiL3NyYy9i
YWNrZW5kL29wdGltaXplci9wbGFuL3BsYW5uZXIuYwppbmRleCA0NDNhYjA4ZDc1Li5lYWJhNmRk
YzAzIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9vcHRpbWl6ZXIvcGxhbi9wbGFubmVyLmMKKysr
IGIvc3JjL2JhY2tlbmQvb3B0aW1pemVyL3BsYW4vcGxhbm5lci5jCkBAIC03MzIwLDYgKzczMjAs
MTUgQEAgZ2F0aGVyX2dyb3VwaW5nX3BhdGhzKFBsYW5uZXJJbmZvICpyb290LCBSZWxPcHRJbmZv
ICpyZWwpCiB7CiAJTGlzdENlbGwgICAqbGM7CiAJUGF0aAkgICAqY2hlYXBlc3RfcGFydGlhbF9w
YXRoOworCUxpc3QJICAgKmdyb3VwYnlfcGF0aGtleXM7CisKKworCS8qIHRyaW0gb2ZmIGFueSBw
YXRoa2V5cyBhZGRlZCBmb3IgT1JERVIgQlkgLyBESVNUSU5DVCBhZ2dyZWdhdGVzICovCisJaWYg
KGxpc3RfbGVuZ3RoKHJvb3QtPmdyb3VwX3BhdGhrZXlzKSA+IHJvb3QtPm51bV9ncm91cGJ5X3Bh
dGhrZXlzKQorCQlncm91cGJ5X3BhdGhrZXlzID0gbGlzdF9jb3B5X2hlYWQocm9vdC0+Z3JvdXBf
cGF0aGtleXMsCisJCQkJCQkJCQkJICByb290LT5udW1fZ3JvdXBieV9wYXRoa2V5cyk7CisJZWxz
ZQorCQlncm91cGJ5X3BhdGhrZXlzID0gcm9vdC0+Z3JvdXBfcGF0aGtleXM7CiAKIAkvKiBUcnkg
R2F0aGVyIGZvciB1bm9yZGVyZWQgcGF0aHMgYW5kIEdhdGhlciBNZXJnZSBmb3Igb3JkZXJlZCBv
bmVzLiAqLwogCWdlbmVyYXRlX3VzZWZ1bF9nYXRoZXJfcGF0aHMocm9vdCwgcmVsLCB0cnVlKTsK
QEAgLTczMzQsNyArNzM0Myw3IEBAIGdhdGhlcl9ncm91cGluZ19wYXRocyhQbGFubmVySW5mbyAq
cm9vdCwgUmVsT3B0SW5mbyAqcmVsKQogCQlpbnQJCQlwcmVzb3J0ZWRfa2V5czsKIAkJZG91Ymxl
CQl0b3RhbF9ncm91cHM7CiAKLQkJaXNfc29ydGVkID0gcGF0aGtleXNfY291bnRfY29udGFpbmVk
X2luKHJvb3QtPmdyb3VwX3BhdGhrZXlzLAorCQlpc19zb3J0ZWQgPSBwYXRoa2V5c19jb3VudF9j
b250YWluZWRfaW4oZ3JvdXBieV9wYXRoa2V5cywKIAkJCQkJCQkJCQkJCXBhdGgtPnBhdGhrZXlz
LAogCQkJCQkJCQkJCQkJJnByZXNvcnRlZF9rZXlzKTsKIApAQCAtNzM2Niw3ICs3Mzc1LDcgQEAg
Z2F0aGVyX2dyb3VwaW5nX3BhdGhzKFBsYW5uZXJJbmZvICpyb290LCBSZWxPcHRJbmZvICpyZWwp
CiAJCQlwYXRoID0gKFBhdGggKikgY3JlYXRlX2luY3JlbWVudGFsX3NvcnRfcGF0aChyb290LAog
CQkJCQkJCQkJCQkJCQkgcmVsLAogCQkJCQkJCQkJCQkJCQkgcGF0aCwKLQkJCQkJCQkJCQkJCQkJ
IHJvb3QtPmdyb3VwX3BhdGhrZXlzLAorCQkJCQkJCQkJCQkJCQkgZ3JvdXBieV9wYXRoa2V5cywK
IAkJCQkJCQkJCQkJCQkJIHByZXNvcnRlZF9rZXlzLAogCQkJCQkJCQkJCQkJCQkgLTEuMCk7CiAK
QEAgLTczNzUsNyArNzM4NCw3IEBAIGdhdGhlcl9ncm91cGluZ19wYXRocyhQbGFubmVySW5mbyAq
cm9vdCwgUmVsT3B0SW5mbyAqcmVsKQogCQkJCQkJCQkJIHJlbCwKIAkJCQkJCQkJCSBwYXRoLAog
CQkJCQkJCQkJIHJlbC0+cmVsdGFyZ2V0LAotCQkJCQkJCQkJIHJvb3QtPmdyb3VwX3BhdGhrZXlz
LAorCQkJCQkJCQkJIGdyb3VwYnlfcGF0aGtleXMsCiAJCQkJCQkJCQkgTlVMTCwKIAkJCQkJCQkJ
CSAmdG90YWxfZ3JvdXBzKTsKIAo=
--0000000000002ed10b061392be13--