Re: Removing unneeded self joins

Hi, Mechael!

On Fri, Jun 27, 2025 at 3:55 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Fri, Jun 27, 2025 at 3:26 AM Michael Paquier <michael@paquier.xyz> wrote:
> > On Thu, Jun 26, 2025 at 08:54:55AM +0200, Andrei Lepikhov wrote:
> > > Before diving into the pg_hint_plan code, I wonder why you don't have
> > > similar issues with the remove_useless_joins. We intentionally designed SJE
> > > coupled with the left-join removal feature to avoid such type of complaints:
> > >
> > > CREATE TABLE test (x integer PRIMARY KEY);
> > > EXPLAIN (COSTS OFF)
> > > SELECT t1.* FROM test t1 LEFT JOIN test t2 ON (t1.x=t2.x);
> > >
> > > /*
> > >      QUERY PLAN
> > > ---------------------
> > >  Seq Scan on test t1
> > > (1 row)
> > > */
> > >
> > > It seems that this join removal is also beyond the pg_hint_plan control ...
> >
> > Yeah, remove_useless_joins() has been around since the 9.0 ages as far
> > as I know, and it's not bothered the module much in the tests because
> > we have unlikely relied on it.
> >
> > For now the trick I am going to rely on is just disable
> > enable_self_join_elimination in the test paths where I rely on the
> > same relation and self joins to keep the plans of the regression tests
> > stable.  This stuff could create separate relations, but then with
> > back-patching in mind that's going to be just extra conflict noise,
> > which is always annoying when dealing with plan outputs.  We've done
> > that in the past with max_parallel_workers_per_gather or jit, to keep
> > the expected plans stable.
> >
> > The point regarding the search join hook may stand, though.  Perhaps
> > somebody should check if we're still OK with this change in the
> > context of the self-join work.  I tend to think that we are and I
> > agree that removing the joins when calling the hook can show benefits,
> > but it may be surprising and users tend to be very noisy with plan
> > stability, particularly if some of the FROM aliases get silenced by
> > the backend without the module knowing about that.  At least there is
> > the trick with SET enable_self_join_elimination available as a last
> > resort method.
>
> One thing I could additionally propose it to add hook, which gets
> called before self-join elimination.  If pg_hint_plan will use this
> hook, it could prevent self-join elimination or track this fact in its
> data structures.
>
> On the other hand we didn't do similar thing for
> remove_useless_joins().  Not sure if it's justified now.

I recently got notification this is in Open Items.
https://wiki.postgresql.org/wiki/PostgreSQL_18_Open_Items
What is your opinion on this: do we need additional hook, fair to
leave this "as is" or another option?

------
Regards,
Alexander Korotkov
Supabase