On Fri, Nov 30, 2012 at 3:19 AM, Andres Freund <and...@2ndquadrant.com>wrote:
> Hi, > > while looking at trigger.c from the POV of foreign key locks I noticed > that GetTupleForTrigger commentless assumes it can just look at a pages > content without a > LockBuffer(buffer, BUFFER_LOCK_SHARE); > > That code path is only reached for AFTER ... FOR EACH ROW ... triggers, > so its fine it's not locking the tuple itself. That already needs to > have happened before. > > The code in question: > > if (newslot_which_is_passed_by_before_triggers) > ... > else > { > Page page; > ItemId lp; > > buffer = ReadBuffer(relation, ItemPointerGetBlockNumber(tid)); > > page = BufferGetPage(buffer); > lp = PageGetItemId(page, ItemPointerGetOffsetNumber(tid)); > > Assert(ItemIdIsNormal(lp)); > > tuple.t_data = (HeapTupleHeader) PageGetItem(page, lp); > tuple.t_len = ItemIdGetLength(lp); > tuple.t_self = *tid; > tuple.t_tableOid = RelationGetRelid(relation); > } > > result = heap_copytuple(&tuple); > ReleaseBuffer(buffer); > > As can be seen in the excerpt above this is basically a very stripped > down heap_fetch(). But without any locking on the buffer we just read. > > I can't believe this is safe? Just about anything but eviction could > happen to that buffer at that moment? > > Am I missing something? > > That seems to be safe to me. Anything thats been read above can't really change. The tuple is already locked, so a concurrent update/delete has to wait on us. We have a pin on the buffer, so VACUUM or HOT-prune can't happen either. I can't see any other operation that can really change those fields. 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. Thanks, Pavan -- Pavan Deolasee http://www.linkedin.com/in/pavandeolasee