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: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>, Richard Guo <guofenglinux@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-01-10T03:13:52Z
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 Mon, Dec 23, 2024 at 10:25 AM Andrei Lepikhov <lepihov@gmail.com> wrote:
> On 12/9/24 13:03, Alexander Korotkov wrote:
> > On Sat, Jul 20, 2024 at 2:38 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > We may decide to generate not just a walker but most of the logic in
> > remove_self_join_rel().  This is probably possible by injecting way
> > more meta-information into definitions of structures.  That isn't
> > going to be simpler than the current approach.  But it is probably
> > less error-prone: one could realize that if you add a new field to the
> > structure, it should have a similar pg_node_attr() as surroundings.
> > But I am afraid that if we go this way, we may end up with an awkward
> > heap of pg_node_attr() to generate the functionality of
> > remove_self_join_rel().  Should we better add comments to PlannerInfo
> > and other relevant structures saying: if you're going to modify this,
> > consider how that affects remove_self_join_rel()?
> >
> > Any thoughts?
>
> Observing positive cases caused by the SJE feature, I resumed the work
> and, following Alexander's suggestion, added developer comments on
> checking remove_self_join_rel in case of the PlannerInfo changes.
>
> I see now that it helps apply after pull-up transformations and,
> sometimes, because of clauses derived from EquivalenceClasses (see the
> patch comment for examples). So, it is not only about dumb ORM-generated
> queries.
>
> Also, multiple code changes since the v7 version caused corresponding
> changes in the SJE code on rebase. So, the new version provided in the
> attachment needs a fresh review.

I've got an off-list report from Alexander Lakhin.  The failing query
is added to the regression tests in the revised patch.  The query was
failing with an error "negative bitmapset member not allowed" issued
in adjust_relid_set().  In order to fix that I've to teach
adjust_relid_set() about negative newrelid, it became even more
similar to replace_relid().  Notable this error happens in
remove_leftjoinrel_from_query(). This seems to be consequence that we
made self-join removal and left-joins removal use common
infrastructure.

------
Regards,
Alexander Korotkov
Supabase