Re: MERGE bug report - Mailing list pgsql-hackers

From Richard Guo
Subject Re: MERGE bug report
Date
Msg-id CAMbWs494uDUNLURxWKA78GaY2MWiKpwo7eHDdmekAje3D=Xc0A@mail.gmail.com
Whole thread Raw
In response to Re: MERGE bug report  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Responses Re: MERGE bug report  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
List pgsql-hackers

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

pgsql-hackers by date:

Previous
From: Peter Geoghegan
Date:
Subject: Re: [PATCH] Full support for index LP_DEAD hint bits on standby
Next
From: Kyotaro Horiguchi
Date:
Subject: Re: API stability