On 25.06.2011 22:29, Kevin Grittner wrote:
> In looking this over I noticed something else that doesn't seem quite
> right. In heapam.c there are two places where the execution of
> PredicateLockTuple() is conditioned not just on MVCC visibility, but
> also on HeapKeyTest(). I think those calls should be moved to not be
> conditioned on that. Otherwise we have a predicate condition being
> tested without "locking the gaps", don't we?
Locks on heap tuples don't serve the purpose of locking gaps, anyway,
because you can't lock anything that doesn't yet exist that way. Locks
on index pages and the heap relation serve that purpose.
I wonder if we need those PredicateLockTuple() calls in heapam.c at all.
You need to take a whole-relation lock on the heap to lock the gaps, to
ensure that you conflict newly inserted rows. And if fetch a tuple via
an index, you acquire locks on the index pages. What is the point of the
PredicateLockTuple() calls? We added the rs_relpredicatelocked mechanism
as an optimization, but I'm thinking that it's actually required for
correctness to grab a predicate lock on the whole relation when you do a
seqscan.
-- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com