From: | Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> |
---|---|
To: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
Cc: | Dmitry <dsy(dot)075(at)yandex(dot)ru>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Inconsistent update in the MERGE command |
Date: | 2025-09-09 13:25:28 |
Message-ID: | 20250909222528.f4a5aa80f2ee5920b542a39c@sraoss.co.jp |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, 5 Sep 2025 08:47:19 +0100
Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
> On Thu, 4 Sept 2025 at 16:03, Yugo Nagata <nagata(at)sraoss(dot)co(dot)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.
Thank you for committing it, and for your additional work on improving
the test and the backpatch.
Regards,
Yugo Nagata
--
Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
From | Date | Subject | |
---|---|---|---|
Next Message | Nazir Bilal Yavuz | 2025-09-09 13:27:47 | Re: Checkpointer write combining |
Previous Message | Robert Haas | 2025-09-09 13:17:51 | Re: plan shape work |