Re: Clean up remove_rel_from_query() after self-join elimination commit

HI Richard
> While working on reusing remove_rel_from_query() for inner-join
> removal, I noticed that the function is in pretty rough shape. The
> self-join elimination (SJE) commit grafted self-join removal onto
> remove_rel_from_query(), which was originally written for left-join
> removal only.

Thanks for working on this.

I see there are already asserts here for the mode split and for ojrelid
validity. What I had in mind was slightly narrower: making the joinrelids
contract explicit as well.As far as I can tell, the existing asserts would
still allow an outer-join call with joinrelids == NULL, even though
joinrelids is later used in the outer-join-specific PHV handling.
I wonder if something like

```c
Assert(!is_outer_join || joinrelids != NULL);
Assert(!is_self_join || joinrelids == NULL);

Thanks

On Tue, Apr 7, 2026 at 4:22 PM Andrei Lepikhov <lepihov(at)gmail(dot)com> wrote:

> On 06/04/2026 10:11, Richard Guo wrote:
> > Thoughts?
> Thanks for your efforts!
>
> The main goal of the SJE feature was to find common ground within the
> community - to come up with a solution that everyone could get behind on
> whether to allow optimisations that address redundant queries and reduce
> query tree complexity in early planning stages. So, some code roughness
> was ok.
>
> I looked through your code, though maybe not as deeply as this part of
> the planner deserves. Overall, it looks good, and I didn’t spot any
> obvious problems, but I do have a few comments. We added some
> ‘redundant’ checks with future optimisations in mind, so others can rely
> on these functions to remove tails from query structures or to error if
> something was left behind.
>
> You explicitly write:
>
> ‘Each specific caller remains responsible for updating any remaining
> data structures required by its unique removal logic’
>
> that differs from the initial idea. At the same time, by the end of SJE
> development, I wasn’t so sure we could invent a universal approach to
> guarantee the cleanup of the query tree - mostly because of the inherent
> volatility of the PlannerInfo structure and because we had not agreed to
> make the parse tree a read-only structure.
>
> So, I’m fine with the changes in this patch.
>
> --
> regards, Andrei Lepikhov,
> pgEdge
>
>
>