Re: BUG #17781: Assert in setrefs.c - Mailing list pgsql-bugs

From Richard Guo
Subject Re: BUG #17781: Assert in setrefs.c
Date
Msg-id CAMbWs4_ngw9sKxpTE8hqk=-ooVX_CQP3DarA4HzkRMz_JKpTrA@mail.gmail.com
Whole thread Raw
In response to Re: BUG #17781: Assert in setrefs.c  (Robins Tharakan <tharakan@gmail.com>)
Responses Re: BUG #17781: Assert in setrefs.c  (Richard Guo <guofenglinux@gmail.com>)
List pgsql-bugs

On Wed, Feb 8, 2023 at 1:52 PM Robins Tharakan <tharakan@gmail.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@sss.pgh.pa.us> wrote:
> PG Bug reporting form <noreply@postgresql.org> writes:
> > This assert() is easily reproducible as of aa69541046@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

pgsql-bugs by date:

Previous
From: Robins Tharakan
Date:
Subject: Re: BUG #17781: Assert in setrefs.c
Next
From: Richard Guo
Date:
Subject: Re: BUG #17781: Assert in setrefs.c