Content-Type: multipart/mixed; boundary="------------cvgbk14C5ufqo4hCo3351rZ2"
Message-ID: <d6319b0c-b186-472f-8b64-41dfa41e7b5f@postgrespro.ru>
Date: Tue, 11 Feb 2025 18:31:33 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Removing unneeded self joins
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 Tom Lane <tgl@sss.pgh.pa.us>, Andrei Lepikhov <lepihov@gmail.com>,
 Robert Haas <robertmhaas@gmail.com>, Richard Guo <guofenglinux@gmail.com>,
 Alexander Lakhin <exclusion@gmail.com>,
 "Gregory Stark (as CFM)" <stark.cfm@gmail.com>,
 =?UTF-8?Q?Micha=C5=82_K=C5=82eczek?= <michal@kleczek.org>,
 Dean Rasheed <dean.a.rasheed@gmail.com>
References: <64486b0b-0404-e39e-322d-0801154901f3@postgrespro.ru>
 <2422119.1714691974@sss.pgh.pa.us>
 <CAPpHfdv6B8HCLdj8WidBryRrX0+X3F1rrR8uAuMQmp6rvPdscg@mail.gmail.com>
 <24e93efd-eeac-43a6-a75a-ad68eacb32ee@postgrespro.ru>
 <CA+TgmoZ66x=uWasH+O40a=G=fpbXN1ANEdGVunvR8BKuekqa2g@mail.gmail.com>
 <3622801.1715010885@sss.pgh.pa.us>
 <CAPpHfduRB9oW3YRDpvPGcDSAbH4iu0LSRhxpRRAzBh7euC9amQ@mail.gmail.com>
 <CAEZATCUeJ93gWCEzK1eRxyZakvv5PVG9LfF3PkSGMsNBC7h9kA@mail.gmail.com>
 <CAPpHfdsj1WdqoCg+g4oq1XQ_=KOj0U4pXqhauBc7e+0msHu66A@mail.gmail.com>
 <CAPpHfds16a7bjQHv-jCLMNzwdukJ_i3da2e0OJFB9L5rTCWEEg@mail.gmail.com>
 <ac7d7334-7ebb-4330-a894-12ba12ffa4ae@gmail.com>
 <CAPpHfdsuc524-BkXNZ+iz9dH=-KiU8qx9CSyE-Nip4Wjz5pGgg@mail.gmail.com>
 <ac34e44e-f8e1-4601-8bb8-fa5e65578832@gmail.com>
 <CAPpHfdvHzExyROoxvVQz6JR4tGVOjgNyHnu8xJmGcx-bMxWvqw@mail.gmail.com>
 <5934ce1e-be02-4640-904c-f7fec3962784@gmail.com>
 <CAPpHfdt9Z=iriS1Fj0Vyo41Z4cS+zUSZM8D3qXyw5LfHr_jVjA@mail.gmail.com>
Content-Language: en-US
From: Alena Rybakina <a.rybakina@postgrespro.ru>
In-Reply-To: 
 <CAPpHfdt9Z=iriS1Fj0Vyo41Z4cS+zUSZM8D3qXyw5LfHr_jVjA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/d6319b0c-b186-472f-8b64-41dfa41e7b5f%40postgrespro.ru>
Precedence: bulk

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

Hi! Thank you for the work with this subject, I think it is really 
important.

On 10.02.2025 22:58, Alexander Korotkov wrote:
> Hi!
>
> On Mon, Feb 10, 2025 at 7:19 AM Andrei Lepikhov <lepihov@gmail.com> wrote:
>> On 9/2/2025 18:41, Alexander Korotkov wrote:
>>> Regarding adjust_relid_set() and replace_relid().  I think they are
>>> now strictly equivalent, except for the case then old relid is given
>>> and not found.  In this case adjust_relid_set() returns the original
>>> relids while replace_relid() returns a copy.  The behavior of
>>> adjust_relid_set() appears more desirable as we don't need extra
>>> copying when no modification is done.  So, I've replaced all
>>> replace_relid() with adjust_relid_set().
>> Ok, I glanced into it, and it makes sense to merge these routines.
>> I think the comment to adjust_relid_set() should be arranged, too. See
>> the attachment for a short variant of such modification.
>>> Also, I did some grammar correction to your new comment in tests.
>> Thanks!
> I've further revised adjust_relid_set() header comment.
>
> Looking back to the work done since previous attempt to commit this to
> pg17, I can highlight following.
> 1) We're now using more of existing infrastructure including
> adjust_relid_set() and ChangeVarNodes().  The most of complexity is
> still there though.
> 2) We've checked few ways to further simplify this patch.  But yet the
> current way still feels to be best possible.
> 3) For sure, several bugs were fixed.
>
> I think we could give it another chance for pg18 after some further
> polishing (at least commit message still needs to be revised).  Any
> thoughts on this?  Tom?
>
I didn't find any mistakes, I just have a refactoring remark. I think 
the part where we add non-redundant expressions with the 
binfo_candidates, jinfo_candidates
check can be moved to a separate function, otherwise the code is very 
repetitive in this place. I did it and attached diff file


-- 
Regards,
Alena Rybakina
Postgres Professional

--------------cvgbk14C5ufqo4hCo3351rZ2
Content-Type: text/x-patch; charset=UTF-8; name="self-join-removal.diff"
Content-Disposition: attachment; filename="self-join-removal.diff"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL29wdGltaXplci9wbGFuL2FuYWx5emVqb2lucy5j
IGIvc3JjL2JhY2tlbmQvb3B0aW1pemVyL3BsYW4vYW5hbHl6ZWpvaW5zLmMKaW5kZXggZDBk
OWI4MGJlOWMuLmQzMTQyNzY5M2RhIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9vcHRpbWl6
ZXIvcGxhbi9hbmFseXplam9pbnMuYworKysgYi9zcmMvYmFja2VuZC9vcHRpbWl6ZXIvcGxh
bi9hbmFseXplam9pbnMuYwpAQCAtMTU3OCw2ICsxNTc4LDQ4IEBAIHJlc3RyaWN0X2luZm9z
X2xvZ2ljYWxseV9lcXVhbChSZXN0cmljdEluZm8gKmEsIFJlc3RyaWN0SW5mbyAqYikKIAly
ZXR1cm4gcmVzdWx0OwogfQogCisvKgorICogVGhlIGZ1bmN0aW9ucyBhZGRzIGFsbCBub24t
cmVkdW5kYW50IGNsYXVzZXMgdG8gdGhlIGtlZXBpbmcgcmVsYXRpb24uCisgKiBDb250cmFk
aWN0b3J5IG9wZXJhdGlvbi4gT24gdGhlIG9uZSBzaWRlLCB3ZSByZWR1Y2UgdGhlIGxlbmd0
aCBvZgorICogcmVzdHJpY3QgbGlzdHMgdGhhdCBjYW4gaW1wYWN0IHBsYW5uaW5nIG9yIGV4
ZWN1dGluZyB0aW1lLgorICogQWRkaXRpb25hbGx5LCB3ZSBpbXByb3ZlIHRoZSBhY2N1cmFj
eSBvZiBjYXJkaW5hbGl0eSBlc3RpbWF0aW9uLiBPbiB0aGUKKyAqIG90aGVyIHNpZGUsIGl0
IGlzIG9uZSBtb3JlIHBsYWNlIHRoYXQgY2FuIG1ha2UgcGxhbm5pbmcgdGltZSBtdWNoCisg
KiBsb25nZXIgaW4gc3BlY2lmaWMgY2FzZXMuIEl0IHdvdWxkIGhhdmUgYmVlbiBiZXR0ZXIg
dG8gYXZvaWQgY2FsbGluZworICogdGhlIGVxdWFsKCkgZnVuY3Rpb24gaGVyZSwgYnV0IGl0
J3MgdGhlIG9ubHkgd2F5IHRvIGRldGVjdCBkdXBsaWNhdGVkCisgKiBpbmVxdWFsaXR5IGV4
cHJlc3Npb25zLgorICovCitzdGF0aWMgdm9pZAorYWRkX25vbl9yZWR1bmRhbnRfY2xhdXNl
cyhQbGFubmVySW5mbyAqcm9vdCwKKwkJCQkJCSAgTGlzdCAqcmluZm9fY2FuZGlkYXRlcywK
KwkJCQkJCSAgTGlzdCAqa2VlcF9yaW5mb19saXN0LAorCQkJCQkJICBJbmRleCByZW1vdmVk
X3JlbGlkKQoreworCWZvcmVhY2hfbm9kZShSZXN0cmljdEluZm8sIHJpbmZvLCByaW5mb19j
YW5kaWRhdGVzKQorCXsKKwkJYm9vbAkJaXNfcmVkdW5kYW50ID0gZmFsc2U7CisKKwkJQXNz
ZXJ0KCFibXNfaXNfbWVtYmVyKHJlbW92ZWRfcmVsaWQsIHJpbmZvLT5yZXF1aXJlZF9yZWxp
ZHMpKTsKKworCQlmb3JlYWNoX25vZGUoUmVzdHJpY3RJbmZvLCBzcmMsIGtlZXBfcmluZm9f
bGlzdCkKKwkJeworCQkJaWYgKCFibXNfZXF1YWwoc3JjLT5jbGF1c2VfcmVsaWRzLCByaW5m
by0+Y2xhdXNlX3JlbGlkcykpCisJCQkJLyogQ2FuJ3QgY29tcGFyZSB0cml2aWFsbHkgZGlm
ZmVyZW50IGNsYXVzZXMgKi8KKwkJCQljb250aW51ZTsKKworCQkJaWYgKHNyYyA9PSByaW5m
byB8fAorCQkJCShyaW5mby0+cGFyZW50X2VjICE9IE5VTEwgJiYKKwkJCQkgc3JjLT5wYXJl
bnRfZWMgPT0gcmluZm8tPnBhcmVudF9lYykgfHwKKwkJCQkgcmVzdHJpY3RfaW5mb3NfbG9n
aWNhbGx5X2VxdWFsKHJpbmZvLCBzcmMpKQorCQkJeworCQkJCWlzX3JlZHVuZGFudCA9IHRy
dWU7CisJCQkJYnJlYWs7CisJCQl9CisJCX0KKwkJaWYgKCFpc19yZWR1bmRhbnQpCisJCQlk
aXN0cmlidXRlX3Jlc3RyaWN0aW5mb190b19yZWxzKHJvb3QsIHJpbmZvKTsKKwl9Cit9CisK
IC8qCiAgKiBSZW1vdmUgYSByZWxhdGlvbiBhZnRlciB3ZSBoYXZlIHByb3ZlbiB0aGF0IGl0
IHBhcnRpY2lwYXRlcyBvbmx5IGluIGFuCiAgKiB1bm5lZWRlZCB1bmlxdWUgc2VsZiBqb2lu
LgpAQCAtMTY1NCw2MiArMTY5NiwxMCBAQCByZW1vdmVfc2VsZl9qb2luX3JlbChQbGFubmVy
SW5mbyAqcm9vdCwgUGxhblJvd01hcmsgKmttYXJrLCBQbGFuUm93TWFyayAqcm1hcmssCiAK
IAkvKgogCSAqIE5vdywgYWRkIGFsbCBub24tcmVkdW5kYW50IGNsYXVzZXMgdG8gdGhlIGtl
ZXBpbmcgcmVsYXRpb24uCi0JICogQ29udHJhZGljdG9yeSBvcGVyYXRpb24uIE9uIHRoZSBv
bmUgc2lkZSwgd2UgcmVkdWNlIHRoZSBsZW5ndGggb2YKLQkgKiByZXN0cmljdCBsaXN0cyB0
aGF0IGNhbiBpbXBhY3QgcGxhbm5pbmcgb3IgZXhlY3V0aW5nIHRpbWUuCi0JICogQWRkaXRp
b25hbGx5LCB3ZSBpbXByb3ZlIHRoZSBhY2N1cmFjeSBvZiBjYXJkaW5hbGl0eSBlc3RpbWF0
aW9uLiBPbiB0aGUKLQkgKiBvdGhlciBzaWRlLCBpdCBpcyBvbmUgbW9yZSBwbGFjZSB0aGF0
IGNhbiBtYWtlIHBsYW5uaW5nIHRpbWUgbXVjaAotCSAqIGxvbmdlciBpbiBzcGVjaWZpYyBj
YXNlcy4gSXQgd291bGQgaGF2ZSBiZWVuIGJldHRlciB0byBhdm9pZCBjYWxsaW5nCi0JICog
dGhlIGVxdWFsKCkgZnVuY3Rpb24gaGVyZSwgYnV0IGl0J3MgdGhlIG9ubHkgd2F5IHRvIGRl
dGVjdCBkdXBsaWNhdGVkCi0JICogaW5lcXVhbGl0eSBleHByZXNzaW9ucy4KIAkgKi8KLQlm
b3JlYWNoX25vZGUoUmVzdHJpY3RJbmZvLCByaW5mbywgYmluZm9fY2FuZGlkYXRlcykKLQl7
Ci0JCWJvb2wJCWlzX3JlZHVuZGFudCA9IGZhbHNlOwotCi0JCUFzc2VydCghYm1zX2lzX21l
bWJlcih0b1JlbW92ZS0+cmVsaWQsIHJpbmZvLT5yZXF1aXJlZF9yZWxpZHMpKTsKLQotCQlm
b3JlYWNoX25vZGUoUmVzdHJpY3RJbmZvLCBzcmMsIHRvS2VlcC0+YmFzZXJlc3RyaWN0aW5m
bykKLQkJewotCQkJaWYgKCFibXNfZXF1YWwoc3JjLT5jbGF1c2VfcmVsaWRzLCByaW5mby0+
Y2xhdXNlX3JlbGlkcykpCi0JCQkJLyogQ29uc3QgYW5kIG5vbi1jb25zdCBleHByZXNzaW9u
cyBjYW4ndCBiZSBlcXVhbCAqLwotCQkJCWNvbnRpbnVlOwotCi0JCQlpZiAoc3JjID09IHJp
bmZvIHx8Ci0JCQkJKHJpbmZvLT5wYXJlbnRfZWMgIT0gTlVMTAotCQkJCSAmJiBzcmMtPnBh
cmVudF9lYyA9PSByaW5mby0+cGFyZW50X2VjKQotCQkJCXx8IHJlc3RyaWN0X2luZm9zX2xv
Z2ljYWxseV9lcXVhbChyaW5mbywgc3JjKSkKLQkJCXsKLQkJCQlpc19yZWR1bmRhbnQgPSB0
cnVlOwotCQkJCWJyZWFrOwotCQkJfQotCQl9Ci0JCWlmICghaXNfcmVkdW5kYW50KQotCQkJ
ZGlzdHJpYnV0ZV9yZXN0cmljdGluZm9fdG9fcmVscyhyb290LCByaW5mbyk7Ci0JfQotCWZv
cmVhY2hfbm9kZShSZXN0cmljdEluZm8sIHJpbmZvLCBqaW5mb19jYW5kaWRhdGVzKQotCXsK
LQkJYm9vbAkJaXNfcmVkdW5kYW50ID0gZmFsc2U7Ci0KLQkJQXNzZXJ0KCFibXNfaXNfbWVt
YmVyKHRvUmVtb3ZlLT5yZWxpZCwgcmluZm8tPnJlcXVpcmVkX3JlbGlkcykpOworCWFkZF9u
b25fcmVkdW5kYW50X2NsYXVzZXMocm9vdCwgYmluZm9fY2FuZGlkYXRlcywgdG9LZWVwLT5i
YXNlcmVzdHJpY3RpbmZvLCB0b1JlbW92ZS0+cmVsaWQpOworCWFkZF9ub25fcmVkdW5kYW50
X2NsYXVzZXMocm9vdCwgamluZm9fY2FuZGlkYXRlcywgdG9LZWVwLT5qb2luaW5mbywgdG9S
ZW1vdmUtPnJlbGlkKTsKIAotCQlmb3JlYWNoX25vZGUoUmVzdHJpY3RJbmZvLCBzcmMsIHRv
S2VlcC0+am9pbmluZm8pCi0JCXsKLQkJCWlmICghYm1zX2VxdWFsKHNyYy0+Y2xhdXNlX3Jl
bGlkcywgcmluZm8tPmNsYXVzZV9yZWxpZHMpKQotCQkJCS8qIENhbid0IGNvbXBhcmUgdHJp
dmlhbGx5IGRpZmZlcmVudCBjbGF1c2VzICovCi0JCQkJY29udGludWU7Ci0KLQkJCWlmIChz
cmMgPT0gcmluZm8gfHwKLQkJCQkocmluZm8tPnBhcmVudF9lYyAhPSBOVUxMCi0JCQkJICYm
IHNyYy0+cGFyZW50X2VjID09IHJpbmZvLT5wYXJlbnRfZWMpCi0JCQkJfHwgcmVzdHJpY3Rf
aW5mb3NfbG9naWNhbGx5X2VxdWFsKHJpbmZvLCBzcmMpKQotCQkJewotCQkJCWlzX3JlZHVu
ZGFudCA9IHRydWU7Ci0JCQkJYnJlYWs7Ci0JCQl9Ci0JCX0KLQkJaWYgKCFpc19yZWR1bmRh
bnQpCi0JCQlkaXN0cmlidXRlX3Jlc3RyaWN0aW5mb190b19yZWxzKHJvb3QsIHJpbmZvKTsK
LQl9CiAJbGlzdF9mcmVlKGJpbmZvX2NhbmRpZGF0ZXMpOwogCWxpc3RfZnJlZShqaW5mb19j
YW5kaWRhdGVzKTsKIAo=

--------------cvgbk14C5ufqo4hCo3351rZ2--