On Wed, 5 Mar 2025 at 09:09, Richard Guo <guofenglinux@gmail.com> wrote:
>
> I noticed the adjacent code that sets wrap_non_vars to true if we
> are considering an appendrel child subquery, and I doubt this is
> necessary.
>
> /*
> * If we are dealing with an appendrel member then anything that's not a
> * simple Var has to be turned into a PlaceHolderVar. We force this to
> * ensure that what we pull up doesn't get merged into a surrounding
> * expression during later processing and then fail to match the
> * expression actually available from the appendrel.
> */
> if (containing_appendrel != NULL)
> rvcontext.wrap_non_vars = true;
>
> As explained in e42e31243, the only part of the upper query that could
> reference the appendrel child yet is the translated_vars list of the
> associated AppendRelInfo that we just made for this child, and we do
> not want to force use of PHVs in the AppendRelInfo (see the comment in
> perform_pullup_replace_vars). In fact, perform_pullup_replace_vars
> sets wrap_non_vars to false before performing pullup_replace_vars on
> the AppendRelInfo. It seems to me that this makes the code shown
> above unnecessary, and we can simply remove it.
>
Yes, that makes sense, and it seems like a fairly straightforward
simplification, given that perform_pullup_replace_vars() sets it back
to false shortly afterwards.
The same thing happens in pull_up_constant_function().
Regards,
Dean
