Re: Removing unneeded self joins

Andrei Lepikhov <lepihov@gmail.com>

From: Andrei Lepikhov <lepihov@gmail.com>
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>, Michał Kłeczek <michal@kleczek.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-07-15T08:31:08Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Remove GUC_NOT_IN_SAMPLE from enable_self_join_elimination

  2. Put enable_self_join_elimination into postgresql.conf.sample

  3. Get rid of ojrelid local variable in remove_rel_from_query()

  4. Implement Self-Join Elimination

  5. Revert: Remove useless self-joins

  6. Replace lateral references to removed rels in subqueries

  7. Replace relids in lateral subquery parse tree during SJE

  8. Forbid SJE with result relation

  9. Fix misuse of RelOptInfo.unique_for_rels cache by SJE

  10. Replace the relid in some missing fields during SJE

  11. Revert 56-bit relfilenode change and follow-up commits.

  12. Stabilize timetz test across DST transitions.

  13. Speed up finding EquivalenceClasses for a given set of rels

  14. Fix mark-and-restore-skipping test case to not be a self-join.

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