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

On Thu, Sep 21, 2023 at 7:49 PM Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
wrote:

> 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.
>

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.

Agreed. We need rowmarks for MERGE too. I found that the following
comment from plannodes.h is very useful in understanding this.

* When doing UPDATE, DELETE, or SELECT FOR UPDATE/SHARE, we have to uniquely
* identify all the source rows, not only those from the target relations, so
* that we can perform EvalPlanQual rechecking at need.

To nitpick, there are some comments that may need to be updated to
include MERGE, such as the comments for ExecRowMark, RowMarkType and
PlanRowMark.

Thanks
Richard