From d3b1b1f9d8d70017bf3e8e4ccf11b183d11389b9 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 14 Jan 2008 01:39:09 +0000
Subject: [PATCH] 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.

---
 src/backend/access/heap/heapam.c | 64 +++++++++++++++++++++++++-------
 src/backend/catalog/index.c      | 18 +++++----
 src/include/access/heapam.h      |  5 ++-
 src/include/access/relscan.h     |  4 +-
 4 files changed, 69 insertions(+), 22 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index cd4637db4b3..7134581333a 100644
--- 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 97904b703cf..a8200b105e4 100644
--- 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 8052e49f30b..78eca9533f7 100644
--- 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 87d8123ba6a..62a0276fd06 100644
--- 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 */
-- 
GitLab