Content-Type: multipart/mixed; boundary="------------JAjz7KDYbwKN5arbLStwqkve"
Message-ID: <73d3a8bb-531d-4db3-be75-e9ba7001a3f3@gmail.com>
Date: Sat, 12 Oct 2024 13:05:25 +0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: BUG #18652: Planner can not find pathkey item to sort for query
 with expression and expression index
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Richard Guo <guofenglinux@gmail.com>, exclusion@gmail.com,
 pgsql-bugs@lists.postgresql.org
References: <18652-deaa782ebcca85d1@postgresql.org>
 <2527690.1728510183@sss.pgh.pa.us>
 <CAMbWs49Pcnm_cX=7azYY9d8+qRrOOKDjJDCa0KGRk9kWWMOz3A@mail.gmail.com>
 <40c8a5c2-0040-4fe6-ab11-6dca23353a07@gmail.com>
 <3001952.1728680364@sss.pgh.pa.us>
Content-Language: en-US
From: Andrei Lepikhov <lepihov@gmail.com>
In-Reply-To: <3001952.1728680364@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/73d3a8bb-531d-4db3-be75-e9ba7001a3f3%40gmail.com>
Precedence: bulk

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

On 12/10/2024 03:59, Tom Lane wrote:
> Andrei Lepikhov <lepihov@gmail.com> writes:
> But that's not what actually happens in the sole use of this code by
> prepare_sort_from_pathkeys.  What will actually happen is that we
> will add the child EC member expression as a new member of the
> same tlist, so that the plan node's tlist will look like
> 
> 	t2.i + 1, (t2.i + 1) + 0
I ended up with the same chain of events. During this scrutiny, I found 
only a few sources of knowledge about why the code was written exactly 
this way and what logic is under the hood. So, the current explanation 
and discussion are profitable by themselves.

> I wonder whether we're doing more work here than we really need to.
> If the underlying table t1 has columns i and j, and we have an EC
> member that references t1.j while the tlist only mentions t1.i,
> wouldn't it still work to add t1.j to the tlist?  So maybe groveling
> through the tlist members is unnecessary and we only need to be
> performing some kind of relation-level check on whether all the
> required relations are included in the input.
I'm not sure I understand this point. Do you mean something like 
attached? I thought about that, but what if we have all the relations 
under, but some columns (especially ones with no source relation) needed 
are absent in the target list and used far below in the plan, or not 
used at all?
The patch looks good.

-- 
regards, Andrei Lepikhov

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

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL29wdGltaXplci9wYXRoL2VxdWl2Y2xhc3MuYyBi
L3NyYy9iYWNrZW5kL29wdGltaXplci9wYXRoL2VxdWl2Y2xhc3MuYwppbmRleCA4ZjZmMDA1
ZWNiLi4xYzBkZWFiMGY2IDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9vcHRpbWl6ZXIvcGF0
aC9lcXVpdmNsYXNzLmMKKysrIGIvc3JjL2JhY2tlbmQvb3B0aW1pemVyL3BhdGgvZXF1aXZj
bGFzcy5jCkBAIC04MzMsMTIgKzg0MSwxMyBAQCBmaW5kX2NvbXB1dGFibGVfZWNfbWVtYmVy
KFBsYW5uZXJJbmZvICpyb290LAogCQkJCQkJICBib29sIHJlcXVpcmVfcGFyYWxsZWxfc2Fm
ZSkKIHsKIAlMaXN0Q2VsbCAgICpsYzsKKwlSZWxpZHMJCXI7CisKKwlyID0gcHVsbF92YXJu
b3Mocm9vdCwgKE5vZGUgKikgZXhwcnMpOwogCiAJZm9yZWFjaChsYywgZWMtPmVjX21lbWJl
cnMpCiAJewogCQlFcXVpdmFsZW5jZU1lbWJlciAqZW0gPSAoRXF1aXZhbGVuY2VNZW1iZXIg
KikgbGZpcnN0KGxjKTsKLQkJTGlzdAkgICAqZXhwcnZhcnM7Ci0JCUxpc3RDZWxsICAgKmxj
MjsKIAogCQkvKgogCQkgKiBXZSBzaG91bGRuJ3QgYmUgdHJ5aW5nIHRvIHNvcnQgYnkgYW4g
ZXF1aXZhbGVuY2UgY2xhc3MgdGhhdApAQCAtODU0LDIxICs4NjMsOCBAQCBmaW5kX2NvbXB1
dGFibGVfZWNfbWVtYmVyKFBsYW5uZXJJbmZvICpyb290LAogCQkJIWJtc19pc19zdWJzZXQo
ZW0tPmVtX3JlbGlkcywgcmVsaWRzKSkKIAkJCWNvbnRpbnVlOwogCi0JCS8qCi0JCSAqIE1h
dGNoIGlmIGFsbCBWYXJzIGFuZCBxdWFzaS1WYXJzIGFyZSBhdmFpbGFibGUgaW4gImV4cHJz
Ii4KLQkJICovCi0JCWV4cHJ2YXJzID0gcHVsbF92YXJfY2xhdXNlKChOb2RlICopIGVtLT5l
bV9leHByLAotCQkJCQkJCQkgICBQVkNfSU5DTFVERV9BR0dSRUdBVEVTIHwKLQkJCQkJCQkJ
ICAgUFZDX0lOQ0xVREVfV0lORE9XRlVOQ1MgfAotCQkJCQkJCQkgICBQVkNfSU5DTFVERV9Q
TEFDRUhPTERFUlMpOwotCQlmb3JlYWNoKGxjMiwgZXhwcnZhcnMpCi0JCXsKLQkJCWlmICgh
aXNfZXhwcmxpc3RfbWVtYmVyKGxmaXJzdChsYzIpLCBleHBycykpCi0JCQkJYnJlYWs7Ci0J
CX0KLQkJbGlzdF9mcmVlKGV4cHJ2YXJzKTsKLQkJaWYgKGxjMikKLQkJCWNvbnRpbnVlOwkJ
CS8qIHdlIGhpdCBhIG5vbi1hdmFpbGFibGUgVmFyICovCisJCWlmICghYm1zX2lzX3N1YnNl
dChlbS0+ZW1fcmVsaWRzLCByKSkKKwkJCWNvbnRpbnVlOwogCiAJCS8qCiAJCSAqIElmIHJl
cXVlc3RlZCwgcmVqZWN0IGV4cHJlc3Npb25zIHRoYXQgYXJlIG5vdCBwYXJhbGxlbC1zYWZl
LiAgV2UKQEAgLTg4NCwzMSArODgwLDYgQEAgZmluZF9jb21wdXRhYmxlX2VjX21lbWJlcihQ
bGFubmVySW5mbyAqcm9vdCwKIAlyZXR1cm4gTlVMTDsKIH0KIAotLyoKLSAqIGlzX2V4cHJs
aXN0X21lbWJlcgotICoJICBTdWJyb3V0aW5lIGZvciBmaW5kX2NvbXB1dGFibGVfZWNfbWVt
YmVyOiBpcyAibm9kZSIgaW4gImV4cHJzIj8KLSAqCi0gKiBQZXIgdGhlIHJlcXVpcmVtZW50
cyBvZiB0aGF0IGZ1bmN0aW9uLCAiZXhwcnMiIG1pZ2h0IG9yIG1pZ2h0IG5vdCBoYXZlCi0g
KiBUYXJnZXRFbnRyeSBzdXBlcnN0cnVjdHVyZS4KLSAqLwotc3RhdGljIGJvb2wKLWlzX2V4
cHJsaXN0X21lbWJlcihFeHByICpub2RlLCBMaXN0ICpleHBycykKLXsKLQlMaXN0Q2VsbCAg
ICpsYzsKLQotCWZvcmVhY2gobGMsIGV4cHJzKQotCXsKLQkJRXhwcgkgICAqZXhwciA9IChF
eHByICopIGxmaXJzdChsYyk7Ci0KLQkJaWYgKGV4cHIgJiYgSXNBKGV4cHIsIFRhcmdldEVu
dHJ5KSkKLQkJCWV4cHIgPSAoKFRhcmdldEVudHJ5ICopIGV4cHIpLT5leHByOwotCi0JCWlm
IChlcXVhbChub2RlLCBleHByKSkKLQkJCXJldHVybiB0cnVlOwotCX0KLQlyZXR1cm4gZmFs
c2U7Ci19Ci0KIC8qCiAgKiByZWxhdGlvbl9jYW5fYmVfc29ydGVkX2Vhcmx5CiAgKgkJQ2Fu
IHRoaXMgcmVsYXRpb24gYmUgc29ydGVkIG9uIHRoaXMgRUMgYmVmb3JlIHRoZSBmaW5hbCBv
dXRwdXQgc3RlcD8K

--------------JAjz7KDYbwKN5arbLStwqkve--