Fix CREATE INDEX CONCURRENTLY so that it won't use synchronized scan for

its second pass over the table.  It has to start at block zero, else the
"merge join" logic for detecting which TIDs are already in the index
doesn't work.  Hence, extend heapam.c's API so that callers can enable or
disable syncscan.  (I put in an option to disable buffer access strategy,
too, just in case somebody needs it.)  Per report from Hannes Dorbath.

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index cd4637db4b3ebd56d439b15d3ac616fca5326900..7134581333a6b550881c565b738b5b9e45ba5b13 100644 (file)
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.247 2008/01/01 19:45:46 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.248 2008/01/14 01:39:09 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -62,6 +62,7 @@
 static HeapScanDesc heap_beginscan_internal(Relation relation,
                                                Snapshot snapshot,
                                                int nkeys, ScanKey key,
+                                               bool allow_strat, bool allow_sync,
                                                bool is_bitmapscan);
 static XLogRecPtr log_heap_update(Relation reln, Buffer oldbuf,
                   ItemPointerData from, Buffer newbuf, HeapTuple newtup, bool move);
@@ -81,6 +82,9 @@ static bool HeapSatisfiesHOTUpdate(Relation relation, Bitmapset *hot_attrs,
 static void
 initscan(HeapScanDesc scan, ScanKey key)
 {
+       bool            allow_strat;
+       bool            allow_sync;
+
        /*
         * Determine the number of blocks we have to scan.
         *
@@ -99,25 +103,39 @@ initscan(HeapScanDesc scan, ScanKey key)
         * strategy and enable synchronized scanning (see syncscan.c).  Although
         * the thresholds for these features could be different, we make them the
         * same so that there are only two behaviors to tune rather than four.
+        * (However, some callers need to be able to disable one or both of
+        * these behaviors, independently of the size of the table.)
         *
         * During a rescan, don't make a new strategy object if we don't have to.
         */
-       if (!scan->rs_bitmapscan &&
-               !scan->rs_rd->rd_istemp &&
+       if (!scan->rs_rd->rd_istemp &&
                scan->rs_nblocks > NBuffers / 4)
+       {
+               allow_strat = scan->rs_allow_strat;
+               allow_sync = scan->rs_allow_sync;
+       }
+       else
+               allow_strat = allow_sync = false;
+
+       if (allow_strat)
        {
                if (scan->rs_strategy == NULL)
                        scan->rs_strategy = GetAccessStrategy(BAS_BULKREAD);
-
-               scan->rs_syncscan = true;
-               scan->rs_startblock = ss_get_location(scan->rs_rd, scan->rs_nblocks);
        }
        else
        {
                if (scan->rs_strategy != NULL)
                        FreeAccessStrategy(scan->rs_strategy);
                scan->rs_strategy = NULL;
+       }
 
+       if (allow_sync)
+       {
+               scan->rs_syncscan = true;
+               scan->rs_startblock = ss_get_location(scan->rs_rd, scan->rs_nblocks);
+       }
+       else
+       {
                scan->rs_syncscan = false;
                scan->rs_startblock = 0;
        }
@@ -1058,29 +1076,47 @@ heap_openrv(const RangeVar *relation, LOCKMODE lockmode)
 /* ----------------
  *             heap_beginscan  - begin relation scan
  *
- * heap_beginscan_bm is an alternative entry point for setting up a HeapScanDesc
- * for a bitmap heap scan.     Although that scan technology is really quite
- * unlike a standard seqscan, there is just enough commonality to make it
- * worth using the same data structure.
+ * heap_beginscan_strat offers an extended API that lets the caller control
+ * whether a nondefault buffer access strategy can be used, and whether
+ * syncscan can be chosen (possibly resulting in the scan not starting from
+ * block zero).  Both of these default to TRUE with plain heap_beginscan.
+ *
+ * heap_beginscan_bm is an alternative entry point for setting up a
+ * HeapScanDesc for a bitmap heap scan.  Although that scan technology is
+ * really quite unlike a standard seqscan, there is just enough commonality
+ * to make it worth using the same data structure.
  * ----------------
  */
 HeapScanDesc
 heap_beginscan(Relation relation, Snapshot snapshot,
                           int nkeys, ScanKey key)
 {
-       return heap_beginscan_internal(relation, snapshot, nkeys, key, false);
+       return heap_beginscan_internal(relation, snapshot, nkeys, key,
+                                                                  true, true, false);
+}
+
+HeapScanDesc
+heap_beginscan_strat(Relation relation, Snapshot snapshot,
+                                        int nkeys, ScanKey key,
+                                        bool allow_strat, bool allow_sync)
+{
+       return heap_beginscan_internal(relation, snapshot, nkeys, key,
+                                                                  allow_strat, allow_sync, false);
 }
 
 HeapScanDesc
 heap_beginscan_bm(Relation relation, Snapshot snapshot,
                                  int nkeys, ScanKey key)
 {
-       return heap_beginscan_internal(relation, snapshot, nkeys, key, true);
+       return heap_beginscan_internal(relation, snapshot, nkeys, key,
+                                                                  false, false, true);
 }
 
 static HeapScanDesc
 heap_beginscan_internal(Relation relation, Snapshot snapshot,
-                                               int nkeys, ScanKey key, bool is_bitmapscan)
+                                               int nkeys, ScanKey key,
+                                               bool allow_strat, bool allow_sync,
+                                               bool is_bitmapscan)
 {
        HeapScanDesc scan;
 
@@ -1103,6 +1139,8 @@ heap_beginscan_internal(Relation relation, Snapshot snapshot,
        scan->rs_nkeys = nkeys;
        scan->rs_bitmapscan = is_bitmapscan;
        scan->rs_strategy = NULL;       /* set in initscan */
+       scan->rs_allow_strat = allow_strat;
+       scan->rs_allow_sync = allow_sync;
 
        /*
         * we can use page-at-a-time mode if it's an MVCC-safe snapshot

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 97904b703cfc6235f1508c895552bed05043416f..a8200b105e456c5fc7bc51fa94b603d54f6a50a0 100644 (file)
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.290 2008/01/03 21:23:15 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.291 2008/01/14 01:39:09 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -2004,12 +2004,16 @@ validate_index_heapscan(Relation heapRelation,
 
        /*
         * Prepare for scan of the base relation.  We need just those tuples
-        * satisfying the passed-in reference snapshot.
-        */
-       scan = heap_beginscan(heapRelation, /* relation */
-                                                 snapshot,             /* seeself */
-                                                 0,    /* number of keys */
-                                                 NULL);        /* scan key */
+        * satisfying the passed-in reference snapshot.  We must disable syncscan
+        * here, because it's critical that we read from block zero forward to
+        * match the sorted TIDs.
+        */
+       scan = heap_beginscan_strat(heapRelation,       /* relation */
+                                                               snapshot,               /* snapshot */
+                                                               0,                              /* number of keys */
+                                                               NULL,                   /* scan key */
+                                                               true,                   /* buffer access strategy OK */
+                                                               false);                 /* syncscan not OK */
 
        /*
         * Scan all tuples matching the snapshot.

diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 8052e49f30b30970a74a1f8a09f96998090b70e3..78eca9533f773eb557caf36198937c0083ae5eb4 100644 (file)
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/access/heapam.h,v 1.129 2008/01/01 19:45:56 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/access/heapam.h,v 1.130 2008/01/14 01:39:09 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -141,6 +141,9 @@ extern Relation heap_openrv(const RangeVar *relation, LOCKMODE lockmode);
 
 extern HeapScanDesc heap_beginscan(Relation relation, Snapshot snapshot,
                           int nkeys, ScanKey key);
+extern HeapScanDesc heap_beginscan_strat(Relation relation, Snapshot snapshot,
+                                        int nkeys, ScanKey key,
+                                        bool allow_strat, bool allow_sync);
 extern HeapScanDesc heap_beginscan_bm(Relation relation, Snapshot snapshot,
                                  int nkeys, ScanKey key);
 extern void heap_rescan(HeapScanDesc scan, ScanKey key);

diff --git a/src/include/access/relscan.h b/src/include/access/relscan.h
index 87d8123ba6a43f0e6204d649e4be4201ee2a5f9c..62a0276fd06f6b1176e5d398d8ad82432b5d643c 100644 (file)
--- a/src/include/access/relscan.h
+++ b/src/include/access/relscan.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/access/relscan.h,v 1.59 2008/01/01 19:45:56 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/access/relscan.h,v 1.60 2008/01/14 01:39:09 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -28,6 +28,8 @@ typedef struct HeapScanDescData
        ScanKey         rs_key;                 /* array of scan key descriptors */
        bool            rs_bitmapscan;  /* true if this is really a bitmap scan */
        bool            rs_pageatatime; /* verify visibility page-at-a-time? */
+       bool            rs_allow_strat; /* allow or disallow use of access strategy */
+       bool            rs_allow_sync;  /* allow or disallow use of syncscan */
 
        /* state set up at initscan time */
        BlockNumber rs_nblocks;         /* number of blocks to scan */