Re: Removing unneeded self joins

On 7/15/24 14:35, jian he wrote:
> On Mon, Jul 15, 2024 at 2:08 PM Andrei Lepikhov <lepihov(at)gmail(dot)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