From: Tom Lane <tgl@sss.pgh.pa.us>
To: Richard Guo <guofenglinux@gmail.com>
cc: tharakan@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17773: Assert triggered on analyzejoins.c
In-reply-to: 
 <CAMbWs4-JSyEcLscQdFCeM=DV2rm0rOk15txJNeWtJeiqb=rXeQ@mail.gmail.com>
References: <17773-a592e6cedbc7bac5@postgresql.org>
 <CAMbWs4-JSyEcLscQdFCeM=DV2rm0rOk15txJNeWtJeiqb=rXeQ@mail.gmail.com>
Comments: In-reply-to Richard Guo <guofenglinux@gmail.com>
	message dated "Mon, 06 Feb 2023 14:34:13 +0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2824903.1675717008.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Mon, 06 Feb 2023 15:56:48 -0500
Message-ID: <2824904.1675717008@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2824904.1675717008%40sss.pgh.pa.us>
Precedence: bulk

Richard Guo <guofenglinux@gmail.com> writes:
> On Mon, Feb 6, 2023 at 1:41 PM PG Bug reporting form <noreply@postgresql=
.org>
> wrote:
>> create table txt();
>> SELECT
>> FROM pg_catalog.pg_roles AS ref_0
>> RIGHT JOIN txt AS ref_1 ON NULL,
>> LATERAL (SELECT
>> WHERE ref_0.rolpassword ~>=3D~ ref_0.rolpassword) AS subq_2;

> It seems something is wrong about the check on PlaceHolderVars in
> outer-join removal codes.  When we want to know if a PHV actually
> references inner-rel's attributes, we check phinfo->ph_var->phexpr with
> pull_varnos.  I suspect this is wrong.  Shouldn't we check
> phinfo->ph_var?

No -- that would destroy the entire point of that bit of code, which
is that ph_eval_at may include a "dummy" reference to the inner rel
that we're hoping to remove.  That happens if we made the PHV's
ph_eval_at equal to its syntactic scope due to its not containing any
Vars.  We can still remove the join, as long as it's possible to
eval the PHV at the join's outer rel instead.

After thinking about this I concluded that the Assert I left behind
yesterday is just wrong.  If we get down to that part of the code,
then we know that it's okay to trim the ph_eval_at values of any such
PHVs --- but if one of them is mentioned in a qual clause then the
clause_relids will also contain those relids.  We have to allow that
during join_is_removable and then remove the relids during
remove_rel_from_query.

It's slightly annoying to lose the cross-checking that those
Asserts used to afford: what if the mention in clause_relids
didn't come from one of the PHVs we're fixing up?  But I don't
see any simple way to re-implement that cross-check.  Adding
a lot of logic to do so would be a bit self-defeating I think;
the extra code might have bugs itself.

			regards, tom lane