On Thu, 4 Sept 2025 at 16:03, Yugo Nagata <nagata@sraoss.co.jp> wrote:
>
> > 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.
>
> ItemPointerIndicatesMovedPartitions() is checked in heapam_tuple_lock(),
> but other table access methods might not perform this check (though I'm
> not sure if this is acceptable), so it may still make sense to keep it.
Good point.
> > 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.
>
> +1
> This should help prevent misuse of the argument in the future.
OK, thanks. Pushed.
In v15 and v16 the code was a little different, and it was actually
much more obvious that it thought that table_tuple_lock() didn't
update the tupleid passed to it, because it was explicitly updating it
from tmfd->ctid.
It was also slightly harder to trigger the error in v15 and v16, so I
had to tweak the tests a little. In doing so, I realised that it's not
actually necessary to have 3 sessions to reproduce this error -- it
only requires 2 sessions, as long as the "other" session does 2
updates. So I did it that way in all branches, which simplified them a
bit.
Regards,
Dean
