Horton

Re: Removing unneeded self joins

Alexander Korotkov <aekorotkov@gmail.com>

View thread
From: Alexander Korotkov <aekorotkov@gmail.com>
To: Andrei Lepikhov <lepihov@gmail.com>
Cc: Richard Guo <guofenglinux@gmail.com>, Ranier Vilela <ranier.vf@gmail.com>, Alena Rybakina <a.rybakina@postgrespro.ru>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>, Alexander Lakhin <exclusion@gmail.com>, "Gregory Stark (as CFM)" <stark.cfm@gmail.com>, Michał Kłeczek <michal@kleczek.org>, Dean Rasheed <dean.a.rasheed@gmail.com>
Date: 2025-04-05T10:09:23Z
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.

Attachments

On Fri, Apr 4, 2025 at 11:35 AM Andrei Lepikhov <lepihov@gmail.com> wrote:
>
> On 4/4/25 04:53, Richard Guo wrote:
> > On Fri, Apr 4, 2025 at 1:02 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> >> I've got an off-list bug report from Alexander Lakhin involving a
> >> placeholder variable.  Alena and Andrei proposed a fix.  It is fairly
> >> simple: we just shouldn't remove PHVs during self-join elimination, as
> >> they might still be referenced from other parts of a query.  The patch
> >> is attached.  I'm going to fix this if no objections.
> >
> > Hmm, I'm not sure about the fix.  It seems to me that it simply
> > prevents removing any PHVs in the self-join removal case.  My concern
> > is that this might result in PHVs that could actually be removed not
> > being removed in many cases.
> Let's play with use cases:
> If a PHV is needed in the inner or outer only, it means we have a clause
> in the baserestrictinfo that will be transferred to the keeping
> relation, and we shouldn't remove the PHV.
> Another case is when the PHV is needed in a join clause of the
> self-join. I may imagine such a case:
>
> toKeep.x+toRemove.y=PHV
>
> This clause will be transformed to "toKeep.x+toKeep.y=PHV", pushed to
> baserestrictinfo of keeping relation and should be saved.
> I think it is possible to invent quite a narrow case of clause like the
> following:
>
> PHV_evaluated_at_inner = PHV_evaluated_at_outer
>
> It needs to prove reproducibility. But even if it makes sense, it seems
> to have no danger for further selectivity estimation compared to the
> source clause and is a too-narrow case, isn't it?
> In other cases, this PHV is needed something else, and we can't remove it.

Should we add more regression tests covering these cases?

> >
> > Besides, there's the specific comment above this code explaining the
> > logic behind the removal of PHVs.  Shouldn't that comment be updated
> > to reflect the changes?
> It makes sense: for now, it seems that PHV removal should be used in the
> case of an outer join removal. In the case of SJE, logically we make a
> replacement, not a removal, and we should not reduce the number of
> entities involved.

I have added a brief comment about that.  Check the attached patch.

------
Regards,
Alexander Korotkov
Supabase