From: Tom Lane <tgl@sss.pgh.pa.us>
To: Richard Guo <guofenglinux@gmail.com>
cc: Robins Tharakan <tharakan@gmail.com>, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17781: Assert in setrefs.c
In-reply-to: 
 <CAMbWs4_aiLNvBw8w2YSZFcU9Z-yKV5pKB+AKsrWJgp8y+r2hcg@mail.gmail.com>
References: <17781-c0405c8b3cd5e072@postgresql.org>
 <3384987.1675812417@sss.pgh.pa.us>
 <CAEP4nAzgCUWd1RWq81Twpud2yAkZejMRqfSoKbvMsdEqHq4KBg@mail.gmail.com>
 <CAMbWs4_ngw9sKxpTE8hqk=-ooVX_CQP3DarA4HzkRMz_JKpTrA@mail.gmail.com>
 <CAMbWs4_aiLNvBw8w2YSZFcU9Z-yKV5pKB+AKsrWJgp8y+r2hcg@mail.gmail.com>
Comments: In-reply-to Richard Guo <guofenglinux@gmail.com>
	message dated "Wed, 08 Feb 2023 17:15:24 +0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3499086.1675876312.1@sss.pgh.pa.us>
Date: Wed, 08 Feb 2023 12:11:52 -0500
Message-ID: <3499087.1675876312@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3499087.1675876312%40sss.pgh.pa.us>
Precedence: bulk

Richard Guo <guofenglinux@gmail.com> writes:
> Hmm. I begin to wonder if it's better to use phinfo->ph_eval_at instead
> in build_joinrel_tlist when we check whether the PHV actually comes from
> the nullable side of an outer join.

I wondered about that too, but I think phrels is probably preferable,
because it is fundamentally a syntactic attribute and what we are
after here is matching the syntactic structure.  I could be convinced
that that's wrong, but I'd want to see an actual counterexample.

> Even so it seems we still need to update phv->phrels in
> remove_rel_from_query when we remove a left join.  Otherwise it'd be
> weird to observe that phrels contains some already-removed relids.

Agreed, that's a good catch.  As the comment for remove_rel_from_query
says,

 * We are not terribly thorough here.

It's not surprising that the requirements for it advance over time.

(At one point I wrote some code that taught it to clean out the
removed relids from EquivalenceClasses, which doesn't need
to happen right now because we don't ever put outer-join quals
into EquivalenceClasses.  I don't think I included that in any
of the published versions of the patch series, but it's going to
need to reappear eventually.)

			regards, tom lane