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

2012-11-30 Thread Pavan Deolasee
On Fri, Nov 30, 2012 at 5:28 PM, Andres Freund and...@2ndquadrant.comwrote: 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

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

2012-11-30 Thread Pavan Deolasee
On Fri, Nov 30, 2012 at 6:21 PM, Andres Freund and...@2ndquadrant.comwrote: On 2012-11-30 18:09:49 +0530, Pavan Deolasee wrote: On Fri, Nov 30, 2012 at 5:28 PM, Andres Freund and...@2ndquadrant.com wrote: That seems to be safe to me. Anything thats been read above can't really

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

2012-11-30 Thread Pavan Deolasee
On Fri, Nov 30, 2012 at 5:28 PM, Andres Freund and...@2ndquadrant.comwrote: 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

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

2012-11-30 Thread Pavan Deolasee
On Fri, Nov 30, 2012 at 6:55 PM, Andres Freund and...@2ndquadrant.comwrote: Hm? It doesn't move the page contents around but it moves the ItemId array during completely normal operation (c.f. needshuffle in PageAddItem). Or am I missing something? I think that probably only used for

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

2012-11-30 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes: On 2012-11-30 14:08:05 -0500, Tom Lane wrote: BTW, I went looking for other places that might have a similar mistake. I found that contrib/pageinspect/btreefuncs.c pokes around in btree pages without any buffer lock, which seems pretty broken ---

[HACKERS] missing LockBuffer(buffer, BUFFER_LOCK_SHARE) in trigger.c GetTupleForTrigger?

2012-11-29 Thread Andres Freund
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

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

2012-11-29 Thread Pavan Deolasee
On Fri, Nov 30, 2012 at 3:19 AM, Andres Freund and...@2ndquadrant.comwrote: 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