Re: MERGE bug report

On Sat, Apr 9, 2022 at 5:26 AM Alvaro Herrera <alvherre@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