From: Tom Lane <tgl@sss.pgh.pa.us>
To: Richard Guo <guofenglinux@gmail.com>
cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>,
        "Finnerty,
    Jim" <jfinnert@amazon.com>
Subject: Re: Making Vars outer-join aware
In-reply-to: 
 <CAMbWs498oYx4pjzOsjxu0wG0D73e2+OFAqkbPotcSJDKzm-s4Q@mail.gmail.com>
References: <830269.1656693747@sss.pgh.pa.us>
 <CAMbWs49KZO2VvMqo+1MQfaX4cun7vhF=5RgVdpA0gjJcj77HFQ@mail.gmail.com>
 <1903934.1657031040@sss.pgh.pa.us> <553080.1657481916@sss.pgh.pa.us>
 <CAMbWs4-0cdVpgY0c2uULBCX2vPjQt3Fx3=XFZh-Mn=UWmxhG-A@mail.gmail.com>
 <1515100.1657633061@sss.pgh.pa.us>
 <CAMbWs4_va6UWcaiPSZpeRPFzpA=UKU+4zKkxqdfrcKoKA5nKWA@mail.gmail.com>
 <2217146.1657721363@sss.pgh.pa.us> <2055912.1659383493@sss.pgh.pa.us>
 <CAMbWs480igxj6hTbvD0WEnzT4XJ8hJXg71EVJO_BcQqN90MQuA@mail.gmail.com>
 <1188905.1660666103@sss.pgh.pa.us> <1405792.1660677844@sss.pgh.pa.us>
 <1538543.1660683428@sss.pgh.pa.us>
 <CAMbWs498oYx4pjzOsjxu0wG0D73e2+OFAqkbPotcSJDKzm-s4Q@mail.gmail.com>
Comments: In-reply-to Richard Guo <guofenglinux@gmail.com>
	message dated "Wed, 17 Aug 2022 17:25:53 +0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2060461.1660767456.1@sss.pgh.pa.us>
Date: Wed, 17 Aug 2022 16:17:36 -0400
Message-ID: <2060462.1660767456@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2060462.1660767456%40sss.pgh.pa.us>
Precedence: bulk

Richard Guo <guofenglinux@gmail.com> writes:
> BTW, the comment just above the two calls to build_joinrel_tlist says:
>  * Create a new tlist containing just the vars that need to be output from
> Here by 'vars' it means both plain Vars and PlaceHolderVars, right? If
> not we may need to adjust this comment to also include PlaceHolderVars.

I think it did intend just Vars because that's all that
build_joinrel_tlist did; but we really should have updated it when we
invented PlaceHolderVars, and even more so now that build_joinrel_tlist
adds PHVs too.  I changed the wording.

> A minor comment is that seems we can get rid of phid inside
> PlaceHolderInfo, since we do not do linear list searches any more. It's
> some duplicate to the phid inside PlaceHolderVar. Currently there are
> two places referencing PlaceHolderInfo->phid, remove_rel_from_query and
> find_placeholder_info. We can use PlaceHolderVar->phid instead in both
> the two places.

Meh, I'm not excited about that.  I don't think that the phid field
is only there to make the search loops faster; it's the basic
identity of the PlaceHolderInfo.

			regards, tom lane