On Thu, 28 Aug 2025 at 15:10, Yugo Nagata <nagata@sraoss.co.jp> wrote:
>
> Thank you for your suggestion and the test patch. The test looks good
> to me, so I’ve attached an updated patch including it.
Thanks, that mostly looks good to me.
There's one other place in ExecMergeMatched() that's using
TM_FailureData.ctid when it shouldn't, and that's this check:
if (ItemPointerIndicatesMovedPartitions(&context->tmfd.ctid))
ereport(ERROR,
(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
errmsg("tuple to be merged was already moved to
another partition due to concurrent update")));
I've updated that to use tupleid in the attached v3 patch, and added a
couple more isolation tests. In practice, however, I don't think that
error can ever happen because this check follows table_tuple_lock()
which has a similar test which will always error out first. I was
tempted to simply remove this check from ExecMergeMatched(), but I
think perhaps it's worth keeping just in case.
Also, I thought that it's worth updating the comments for
table_tuple_lock() and TM_FailureData to make it clearer that it
updates its tid argument, and which fields of TM_FailureData can be
relied upon in the different cases.
Regards,
Dean