Re: missing LockBuffer(buffer, BUFFER_LOCK_SHARE) in trigger.c GetTupleForTrigger?

On Fri, Nov 30, 2012 at 5:28 PM, Andres Freund <andres(at)2ndquadrant(dot)com>wrote:

>
>
> > heap_fetch() though looks very similar has an important difference. It
> > might be reading the tuple without lock on it and we need the buffer lock
> > to protect against concurrent update/delete to the tuple. AFAIK once the
> > tuple is passed through qualification checks etc, even callers of
> > heap_fetch() can read the tuple data without any lock on the buffer
> itself.
>
> Sure, but the page header isn't accessed there, just the tuple data
> itself which always stays at the same place on the page.
> (A normal heap_fetch shouldn't be worried about updates/deletions to its
> tuple, MVCC to the rescue.)
>
>
heap_fetch() reads the tuple header though to apply snapshot visibility
rules. So a SHARE lock is must for that purpose because a concurrent
update/delete may set XMAX or other visibility related fields. On the other
hand, you can read the tuple body without a page lock if you were holding a
pin on the buffer continuously since you last dropped the lock.

In any case, a lock is needed in GetTupleForTrigger unless someone can
argue that a pin is continuously held on the buffer since the lock was last
dropped, something I don't see happening in this case.

Thanks,
Pavan

--
Pavan Deolasee
http://www.linkedin.com/in/pavandeolasee