Re: BUG #18103: bugs of concurrent merge into when use different join plan

On Mon, 18 Sept 2023 at 09:51, Richard Guo <guofenglinux@gmail.com> wrote:
>
> On Thu, Sep 14, 2023 at 3:30 PM David Rowley <dgrowleyml@gmail.com> wrote:
>>
>> I agree this is a bug.
>>
>> What seems to be going on is that in ExecMergeMatched() we hit the
>> TM_Updated case and when we try to fill the epqslot calling
>> EvalPlanQual() the nested loop does not seem to scan to find the
>> correct set of rows.
>
> I have the same observation.  It seems that the EvalPlanQual recheck
> does not work well with LEFT joins if there is concurrent update in a
> WHEN MATCHED case.  FWIW, if the optimizer chooses RIGHT join at last
> for the MERGE command, we'd get the expected results.
>

I don't feel that I have a good understanding of the EPQ mechanism,
but src/backend/executor/README says:

"""
It is also possible that there are relations in the query that are not
to be locked (they are neither the UPDATE/DELETE target nor specified to
be locked in SELECT FOR UPDATE/SHARE).  When re-running the test query
we want to use the same rows from these relations that were joined to
the locked rows.  For ordinary relations this can be implemented relatively
cheaply by including the row TID in the join outputs and re-fetching that
TID.  (The re-fetch is expensive, but we're trying to optimize the normal
case where no re-test is needed.)  We have also to consider non-table
relations, such as a ValuesScan or FunctionScan.  For these, since there
is no equivalent of TID, the only practical solution seems to be to include
the entire row value in the join output row.
"""

and this is not happening for MERGE. For example, given this setup:

drop table if exists t1, t2;
create table t1 (id int primary key, val int);
create table t2 (id int primary key, val int);

an UPDATE, joining t2 to t1 does the following:

explain (verbose, costs off)
update t1 set val = t2.val from t2 where t2.id = t1.id;

                     QUERY PLAN
----------------------------------------------------
 Update on public.t1
   ->  Hash Join
         Output: t2.val, t1.ctid, t2.ctid
         Inner Unique: true
         Hash Cond: (t1.id = t2.id)
         ->  Seq Scan on public.t1
               Output: t1.ctid, t1.id
         ->  Hash
               Output: t2.val, t2.ctid, t2.id
               ->  Seq Scan on public.t2
                     Output: t2.val, t2.ctid, t2.id
(11 rows)

but the equivalent MERGE does not include CTIDs from t2 in the join output:

explain (verbose, costs off)
merge into t1 using t2 on t2.id = t1.id
  when matched then update set val = t2.val;

                QUERY PLAN
-------------------------------------------
 Merge on public.t1
   ->  Hash Join
         Output: t1.ctid, t2.val
         Inner Unique: true
         Hash Cond: (t1.id = t2.id)
         ->  Seq Scan on public.t1
               Output: t1.ctid, t1.id
         ->  Hash
               Output: t2.val, t2.id
               ->  Seq Scan on public.t2
                     Output: t2.val, t2.id
(11 rows)

On the face of it, that looks like a simple oversight in
preprocess_rowmarks(), and changing it, as in the attached, fixes the
reported issue.

Having said that, I'm not sure what guarantees we can really give
about the concurrent behaviour of MERGE. For example, if the source
table contained rows not present in the target, a MERGE with an INSERT
action would be basically the same as an INSERT ... WHERE NOT EXISTS
(...) with nothing to prevent concurrent sessions from inserting
duplicate rows. For concurrent updates, INSERT ... ON CONFLICT DO
UPDATE is probably a better choice.

Regards,
Dean