Message-ID: <ebe66358-428c-439f-85dd-da72c9b2b0c2@postgrespro.ru>
Date: Wed, 4 Oct 2023 13:55:59 +0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Removing unneeded self joins
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: "Gregory Stark (as CFM)" <stark.cfm@gmail.com>,
 =?UTF-8?Q?Micha=C5=82_K=C5=82eczek?= <michal@kleczek.org>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <64486b0b-0404-e39e-322d-0801154901f3@postgrespro.ru>
 <13021914.uLZWGnKmhe@aivenronan>
 <e726cc25-de80-3818-66b2-1b893bb54602@postgrespro.ru>
 <2375492.jE0xQCEvom@aivenronan>
 <CALNJ-vSeE9xzeFK9gM+kNqNF1csM1sH2epeeTZUbS6WFhXraGA@mail.gmail.com>
 <CALNJ-vS4PtC0e53=8fTEe8t3U5d4cTOJYL1PAQsn4=aBk9C5tQ@mail.gmail.com>
 <CALNJ-vTyL-LpvSOPZxpC63Et3LJLUAFZSfRqGEhT5Rj7_EEj7w@mail.gmail.com>
 <0fd15c10-eaa5-51bd-b5c7-e635fe03a94d@postgrespro.ru>
 <d9493d5a-b950-465e-f09b-62c92a23fba1@postgrespro.ru>
 <20221206184643.zya44h5auv6f45zp@awork3.anarazel.de>
 <36579124-f9d9-a51e-38b3-af3511921c7e@postgrespro.ru>
 <88079CBD-B5BB-4C61-A68B-A8C67599E1C7@kleczek.org>
 <CAM-w4HMZXW+hKc8C+WLQ+gdpEkoh9378a2UGKyZRz7mZVerG+Q@mail.gmail.com>
 <c97574c4-13e6-9107-26ab-af2bd626fc85@postgrespro.ru>
 <bcf2f3c5-fbdc-f85b-af3e-ebec616160f6@postgrespro.ru>
 <05469e80-454b-41d7-d04f-421f42bbd4a9@postgrespro.ru>
 <CAPpHfdt9G3EU+vXuqexhweEQoMF-acPUUFuB6hV2Kg0gCy7AjA@mail.gmail.com>
Content-Language: en-US
From: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
Organization: Postgres Professional
In-Reply-To: 
 <CAPpHfdt9G3EU+vXuqexhweEQoMF-acPUUFuB6hV2Kg0gCy7AjA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/ebe66358-428c-439f-85dd-da72c9b2b0c2%40postgrespro.ru>
Precedence: bulk

On 4/10/2023 07:12, Alexander Korotkov wrote:
> Hi!
Thanks for the review!
> 
> I think this is a neat optimization.
> 
> On Tue, Sep 12, 2023 at 4:58 PM Andrey Lepikhov
> <a.lepikhov@postgrespro.ru> wrote:
>> On 5/7/2023 21:28, Andrey Lepikhov wrote:
>>> During the significant code revision in v.41 I lost some replacement
>>> operations. Here is the fix and extra tests to check this in the future.
>>> Also, Tom added the JoinDomain structure five months ago, and I added
>>> code to replace relids for that place too.
>>> One more thing, I found out that we didn't replace SJs, defined by
>>> baserestrictinfos if no one self-join clause have existed for the join.
>>> Now, it is fixed, and the test has been added.
>>> To understand changes readily, see the delta file in the attachment.
>> Here is new patch in attachment. Rebased on current master and some
>> minor gaffes are fixed.
> 
> I went through the thread and I think the patch gets better shape.  A
> couple of notes from my side.
> 1) Why replace_relid() makes a copy of lids only on insert/replace of
> a member, but performs deletion in-place?

Shortly speaking, it was done according to the 'Paranoid' strategy.
The main reason for copying before deletion was the case with the rinfo 
required_relids and clause_relids. They both point to the same Bitmapset 
in some cases. And we feared such things for other fields.
Right now, it may be redundant because we resolved the issue mentioned 
above in replace_varno_walker.

Relid replacement machinery is the most contradictory code here. We used 
a utilitarian approach and implemented a simplistic variant.

> 2) It would be nice to skip the insertion of IS NOT NULL checks when
> they are not necessary.  [1] points that infrastructure from [2] might
> be useful.  The patchset from [2] seems committed mow.  However, I
> can't see it is directly helpful in this matter.  Could we just skip
> adding IS NOT NULL clause for the columns, that have
> pg_attribute.attnotnull set?
Thanks for the links, I will look into that case.
> 
> Links
> 1. https://www.postgresql.org/message-id/2375492.jE0xQCEvom%40aivenronan
> 2.  https://www.postgresql.org/message-id/flat/830269.1656693747%40sss.pgh.pa.us

-- 
regards,
Andrey Lepikhov
Postgres Professional