Message-ID: <3d9457cc-a440-48a5-bf44-8eb3f65316f7@gmail.com>
Date: Mon, 15 Jul 2024 15:31:08 +0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Removing unneeded self joins
To: jian he <jian.universality@gmail.com>
Cc: Alexander Korotkov <aekorotkov@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
 Robert Haas <robertmhaas@gmail.com>,
 Andrei Lepikhov <a.lepikhov@postgrespro.ru>,
 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>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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>
 <CACJufxG3sqJKe1OskHhn7OCdtrEeeRFcD8R4TTQE+LGJEQaL9w@mail.gmail.com>
 <CACJufxFNDukHU2WG7T61orXdgFJ5gEkSVWcgsiGbb8tRL5=DTQ@mail.gmail.com>
 <96250a42-20e3-40f0-9d45-f53ae852f8ed@gmail.com>
 <CACJufxF-ubw739W+33_rFSA+YTdRTDcKwVidKwXAS8NCy4nx+Q@mail.gmail.com>
 <5b49501c-9cb3-4c5d-9d56-49704ff08143@gmail.com>
 <CAPpHfdvTHuhB5+Eohb-z7zWM6u9Y=W6vb8MMOqhfKeRkhVZGxg@mail.gmail.com>
 <CAPpHfdus-O3Ozpn+tFL8r5KPiedaBYSrLK_3BsvJSC7sVBi+_g@mail.gmail.com>
 <CACJufxHBLhOD1LerM643dgh=UZFGhPWfP1027D2x1W6DhF_BaQ@mail.gmail.com>
 <6da9bdb2-3ff2-4b8b-818a-8052772f2163@gmail.com>
 <CACJufxFWiiiSzyB3B2MeMv3wxqTShHyw4mnwwAwGKC0K6VZM+A@mail.gmail.com>
Content-Language: en-AU
From: Andrei Lepikhov <lepihov@gmail.com>
In-Reply-To: 
 <CACJufxFWiiiSzyB3B2MeMv3wxqTShHyw4mnwwAwGKC0K6VZM+A@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/3d9457cc-a440-48a5-bf44-8eb3f65316f7%40gmail.com>
Precedence: bulk

On 7/15/24 14:35, jian he wrote:
> On Mon, Jul 15, 2024 at 2:08 PM Andrei Lepikhov <lepihov@gmail.com> wrote:
>>
>> On 7/15/24 12:31, jian he wrote:
>>> hi.
>>> Here is the latest patch (v6),
>>> I've made the following changes.
>>>
>>> * disallow original Query->resultRelation participate in SJE.
>>> for SELECT, nothing is changed. for UPDATE/DELETE/MERGE
>>> we can do:
>>> EXPLAIN (COSTS OFF)
>>> UPDATE sj sq SET b = sq.b + sz.a FROM (select s1.* from sj s1 join sj
>>> s2 on s1.a = s2.a) as sz
>>> WHERE sz.a = sq.a;
>>>
>>> here, only "(select s1.* from sj s1 join sj s2 on s1.a = s2.a)" can
>>> apply to SJE.
>>>
>>> but for now we cannot apply SJE to
>>> EXPLAIN (COSTS OFF)
>>> UPDATE sj sq SET b = sq.b + sz.a FROM sj as sz WHERE sz.a = sq.a;
>>>
>>> so the EPQ abnormality issue[1] won't happen.
>>>
>>>
>>> * add a new function: ChangeVarNodesExtended for
>>> address concerns in  [2]
>> I see you still stay with the code line:
>> if (omark && imark && omark->markType != imark->markType)
>>
>> It is definitely an error. What if omark is NULL, but imark is not? Why
>> not to skip this pair of relids? Or, at least, insert an assertion to
>> check that you filtered it earlier.
>>
> 
> i  think "omark is NULL, but imark is not" case won't reach to
> remove_self_joins_one_group.
> In that case, omark associated RangeTblEntry->rtekind will be RTE_SUBQUERY,
> and will be skipped earlier in remove_self_joins_recurse.
> 
> 
> Still, do you think the following code is the right way to go?
> 
> if ((omark == NULL && imark != NULL) ||
> (omark != NULL && imark == NULL) ||
> (omark && imark && omark->markType != imark->markType))
>    continue;
Sure, if query block needs RowMark it applies proper RowMark to each 
base relation. All pull-up transformations executes before this code.
But it is worth to set Assert at the point to check that nothing changed 
in the code above and the patch works correctly, am I wrong?


-- 
regards, Andrei Lepikhov