Richard Guo <guofengli...@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
