On Sat, Apr 9, 2022 at 5:26 AM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote:
> On 2022-Apr-06, Richard Guo wrote: > > > That's right. The varattno is set to zero for whole-row Var. And in this > > case these whole-row Vars are not included in the targetlist. > > > > Attached is an attempt for the fix. > > Wow, this is very interesting. I was surprised that this patch was > necessary at all -- I mean, if wholerow refs don't work, then why do > references to any other columns work? The answer is that parse_merge.c > is already setting up the subplan's targetlist by expanding all vars of > the source relation. I then remembered than in Simon's (or Pavan's) > original coding, parse_merge.c had a hack to include a var with the > source's wholerow in that targetlist, which I had later removed ... > At first I was wondering whether we need to also include vars used in each action's targetlist, just as what we did for each action's qual. Then later I realized parse_merge.c already did that. But now it looks much better to process them two in preprocess_targetlist. > > I eventually realized that there's no need for parse_merge.c to expand > the source rel at all, and indeed it's wasteful: we can just let > preprocess_targetlist include the vars that are referenced by either > quals or each action's targetlist instead. That led me to the attached > patch, which is not commit-quality yet but it should show what I have in > mind. > This patch looks in a good shape to me. A minor comment is that we can use list_concat_copy(list1, list2) instead of list_concat(list_copy(list1), list2) for better efficiency. Thanks Richard
