Move the PredicateLockRelation() call from nodeSeqscan.c to heapam.c. It's

more consistent that way, since all the other PredicateLock* calls are
made in various heapam.c and index AM functions. The call in nodeSeqscan.c
was unnecessarily aggressive anyway, there's no need to try to lock the
relation every time a tuple is fetched, it's enough to do it once.

This has the user-visible effect that if a seq scan is initialized in the
executor, but never executed, we now acquire the predicate lock on the heap
relation anyway. We could avoid that by taking the lock on the first
heap_getnext() call instead, but it doesn't seem worth the trouble given
that it feels more natural to do it in heap_beginscan().

Also, remove the retail PredicateLockTuple() calls from heap_getnext(). In
a seqscan, started with heap_begin(), we're holding a whole-relation
predicate lock on the heap so there's no need to lock the tuples
individually.

Kevin Grittner and me

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index b947c11f7d80bd6aed2174bcdffa7cf382707168..450bb15ff8d7a525e1d5b0737f6f9149b19fa324 100644 (file)
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -479,8 +479,6 @@ heapgettup(HeapScanDesc scan,
 
                                if (valid)
                                {
-                                       if (!scan->rs_relpredicatelocked)
-                                               PredicateLockTuple(scan->rs_rd, tuple, snapshot);
                                        LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
                                        return;
                                }
@@ -748,16 +746,12 @@ heapgettup_pagemode(HeapScanDesc scan,
                                                        nkeys, key, valid);
                                if (valid)
                                {
-                                       if (!scan->rs_relpredicatelocked)
-                                               PredicateLockTuple(scan->rs_rd, tuple, scan->rs_snapshot);
                                        scan->rs_cindex = lineindex;
                                        return;
                                }
                        }
                        else
                        {
-                               if (!scan->rs_relpredicatelocked)
-                                       PredicateLockTuple(scan->rs_rd, tuple, scan->rs_snapshot);
                                scan->rs_cindex = lineindex;
                                return;
                        }
@@ -1224,13 +1218,26 @@ heap_beginscan_internal(Relation relation, Snapshot snapshot,
        scan->rs_strategy = NULL;       /* set in initscan */
        scan->rs_allow_strat = allow_strat;
        scan->rs_allow_sync = allow_sync;
-       scan->rs_relpredicatelocked = false;
 
        /*
         * we can use page-at-a-time mode if it's an MVCC-safe snapshot
         */
        scan->rs_pageatatime = IsMVCCSnapshot(snapshot);
 
+       /*
+        * For a seqscan in a serializable transaction, acquire a predicate lock
+        * on the entire relation. This is required not only to lock all the
+        * matching tuples, but also to conflict with new insertions into the
+        * table. In an indexscan, we take page locks on the index pages covering
+        * the range specified in the scan qual, but in a heap scan there is
+        * nothing more fine-grained to lock. A bitmap scan is a different story,
+        * there we have already scanned the index and locked the index pages
+        * covering the predicate. But in that case we still have to lock any
+        * matching heap tuples.
+        */
+       if (!is_bitmapscan)
+               PredicateLockRelation(relation, snapshot);
+
        /* we only need to set this up once */
        scan->rs_ctup.t_tableOid = RelationGetRelid(relation);
 

diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c
index f356874b4412dd5f8ed692da3b08f2ff7640d1f9..0f3438d0639d8c454f5b83851281817e2629a08d 100644 (file)
--- a/src/backend/executor/nodeSeqscan.c
+++ b/src/backend/executor/nodeSeqscan.c
@@ -28,7 +28,6 @@
 #include "access/relscan.h"
 #include "executor/execdebug.h"
 #include "executor/nodeSeqscan.h"
-#include "storage/predicate.h"
 
 static void InitScanRelation(SeqScanState *node, EState *estate);
 static TupleTableSlot *SeqNext(SeqScanState *node);
@@ -106,16 +105,11 @@ SeqRecheck(SeqScanState *node, TupleTableSlot *slot)
  *             tuple.
  *             We call the ExecScan() routine and pass it the appropriate
  *             access method functions.
- *             For serializable transactions, we first acquire a predicate
- *             lock on the entire relation.
  * ----------------------------------------------------------------
  */
 TupleTableSlot *
 ExecSeqScan(SeqScanState *node)
 {
-       PredicateLockRelation(node->ss_currentRelation,
-                                                 node->ss_currentScanDesc->rs_snapshot);
-       node->ss_currentScanDesc->rs_relpredicatelocked = true;
        return ExecScan((ScanState *) node,
                                        (ExecScanAccessMtd) SeqNext,
                                        (ExecScanRecheckMtd) SeqRecheck);

diff --git a/src/include/access/relscan.h b/src/include/access/relscan.h
index 7663033723b413c0fb6c035a5b9f68e4e9d815cf..ee60ca82ae826f1b919700b49a97b126dc1f629f 100644 (file)
--- a/src/include/access/relscan.h
+++ b/src/include/access/relscan.h
@@ -35,7 +35,6 @@ typedef struct HeapScanDescData
        BlockNumber rs_startblock;      /* block # to start at */
        BufferAccessStrategy rs_strategy;       /* access strategy for reads */
        bool            rs_syncscan;    /* report location to syncscan logic? */
-       bool            rs_relpredicatelocked;  /* predicate lock on relation exists */
 
        /* scan current state */
        bool            rs_inited;              /* false = scan not init'd yet */