Peter Geoghegan <> writes:
> Attached is a revision of Thomas' patch to fix a related issue; it now
> also fixes this no-buffer-lock-held issue.

I think this needs more work.

1. Why are you replacing ExecOnConflictUpdate's ExecCheckHeapTupleVisible
call with ExecCheckTIDVisible?  That appears to demand a re-fetch of the
tuple ExecOnConflictUpdate already has its hands on.  Wouldn't it be
better to just put a buffer lock/unlock around the existing code?

2. Your proposed coding

+       if (!heap_fetch(rel, estate->es_snapshot, &tuple, &buffer, true, NULL))
+ ...
+               if 

will dump core in the case where heap_fetch returns false with
tuple.t_data set to null.  If that case is impossible here,
it's not apparent why, and it'd surely deserve at least a comment,
maybe an Assert.  But I'd rather just assume it's possible.

I'm not taking a position on whether this is OK at the higher level
of whether the SSI behavior could be better.  However, unless Kevin
has objections to committing something like this now, I think we
should fix the above-mentioned problems and push it.  The existing
behavior is clearly bad.

                        regards, tom lane

Sent via pgsql-hackers mailing list (
To make changes to your subscription:

Reply via email to