On Wed, 5 Mar 2025 at 09:09, Richard Guo <guofengli...@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
