Re: BUG #17781: Assert in setrefs.c

On Wed, Feb 8, 2023 at 1:52 PM Robins Tharakan <tharakan(at)gmail(dot)com> wrote:

> Hi Tom,
>
> Assuming this persistence is helpful overall, now I see a
> different SQL can still trigger the same assert().
>
> On Wed, 8 Feb 2023 at 09:56, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > PG Bug reporting form <noreply(at)postgresql(dot)org> writes:
> > > This assert() is easily reproducible as of aa69541046(at)master,
> although I can
> > > trace the issue back to last week's commit 2489d76c49.
> >
> > Pushed a fix, thanks!
>
>
> TRAP: failed Assert("nrm_match == NRM_SUBSET ?
> bms_is_subset(phv->phnullingrels, subphv->phnullingrels) : nrm_match
> == NRM_SUPERSET ? bms_is_subset(subphv->phnullingrels,
> phv->phnullingrels) : bms_equal(subphv->phnullingrels,
> phv->phnullingrels)"), File: "setrefs.c", Line: 2845, PID: 803757
>
>
> SQL
> ===
> rollback;
> begin;
> create table t();
> SELECT ref_1.definition AS c4
> FROM t AS sample_1
> LEFT JOIN pg_catalog.pg_rules AS ref_1 ON NULL
> WHERE pg_catalog."user"() IS NOT NULL;

I've looked at this a little bit and I think it's a different issue. It
seems when we've decided that a left join can be removed, we neglect to
remove references to the target rel from PlaceHolderVar->phrels in
remove_rel_from_query. And it turns out that PlaceHolderVar->phrels is
used later in build_joinrel_tlist to check whether the PHV actually
comes from the nullable side of an outer join. I verified that with the
following change and it seems can fix this query.

--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -429,11 +429,16 @@ remove_rel_from_query(PlannerInfo *root, int relid,
int ojrelid,
}
else
{
+ PlaceHolderVar *phv = phinfo->ph_var;
+
phinfo->ph_eval_at = bms_del_member(phinfo->ph_eval_at, relid);
phinfo->ph_eval_at = bms_del_member(phinfo->ph_eval_at,
ojrelid);
Assert(!bms_is_empty(phinfo->ph_eval_at));
phinfo->ph_needed = bms_del_member(phinfo->ph_needed, relid);
phinfo->ph_needed = bms_del_member(phinfo->ph_needed, ojrelid);
+
+ phv->phrels = bms_del_member(phv->phrels, relid);
+ phv->phrels = bms_del_member(phv->phrels, ojrelid);
}

Thanks
Richard