Re: Removing unneeded self joins

On Mon, Dec 23, 2024 at 10:25 AM Andrei Lepikhov <lepihov(at)gmail(dot)com> wrote:
> On 12/9/24 13:03, Alexander Korotkov wrote:
> > On Sat, Jul 20, 2024 at 2:38 PM Alexander Korotkov <aekorotkov(at)gmail(dot)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