From ecb145091ad8ebc48fe25d55c76a2663a9c1ef52 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Thu, 9 Mar 2017 15:50:27 -0800
Subject: [PATCH 2/2] Cleanups & tests.

---
 contrib/amcheck/Makefile                 |   4 +-
 contrib/amcheck/amcheck.control          |   2 +-
 contrib/amcheck/expected/check.out       |   1 +
 contrib/amcheck/expected/check_btree.out |  87 ++++++++++++++++++++++
 contrib/amcheck/sql/check.sql            |   1 +
 contrib/amcheck/sql/check_btree.sql      |  59 +++++++++++++++
 contrib/amcheck/verify_nbtree.c          | 119 +++++++++++++++++--------------
 doc/src/sgml/amcheck.sgml                |  14 ++--
 8 files changed, 223 insertions(+), 64 deletions(-)
 create mode 100644 contrib/amcheck/expected/check.out
 create mode 100644 contrib/amcheck/expected/check_btree.out
 create mode 100644 contrib/amcheck/sql/check.sql
 create mode 100644 contrib/amcheck/sql/check_btree.sql

diff --git a/contrib/amcheck/Makefile b/contrib/amcheck/Makefile
index f14c7aa58b..43bed919ae 100644
--- a/contrib/amcheck/Makefile
+++ b/contrib/amcheck/Makefile
@@ -5,7 +5,9 @@ OBJS		= verify_nbtree.o $(WIN32RES)
 
 EXTENSION = amcheck
 DATA = amcheck--1.0.sql
-PGFILEDESC = "amcheck - functions to verify access method invariants"
+PGFILEDESC = "amcheck - function for verifying relation integrity"
+
+REGRESS = check check_btree
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
diff --git a/contrib/amcheck/amcheck.control b/contrib/amcheck/amcheck.control
index 180f150e7f..05e2861d7a 100644
--- a/contrib/amcheck/amcheck.control
+++ b/contrib/amcheck/amcheck.control
@@ -1,5 +1,5 @@
 # amcheck extension
-comment = 'verify access method invariants'
+comment = 'functions for verifying relation integrity'
 default_version = '1.0'
 module_pathname = '$libdir/amcheck'
 relocatable = true
diff --git a/contrib/amcheck/expected/check.out b/contrib/amcheck/expected/check.out
new file mode 100644
index 0000000000..5b3e6d530c
--- /dev/null
+++ b/contrib/amcheck/expected/check.out
@@ -0,0 +1 @@
+CREATE EXTENSION amcheck;
diff --git a/contrib/amcheck/expected/check_btree.out b/contrib/amcheck/expected/check_btree.out
new file mode 100644
index 0000000000..851b5cdcc1
--- /dev/null
+++ b/contrib/amcheck/expected/check_btree.out
@@ -0,0 +1,87 @@
+-- minimal test, basically just verifying that amcheck
+CREATE TABLE bttest_a(id int8);
+CREATE TABLE bttest_b(id int8);
+INSERT INTO bttest_a SELECT * FROM generate_series(1, 100000);
+INSERT INTO bttest_b SELECT * FROM generate_series(100000, 1, -1);
+CREATE INDEX bttest_a_idx ON bttest_a USING btree (id);
+CREATE INDEX bttest_b_idx ON bttest_b USING btree (id);
+CREATE ROLE bttest_role;
+-- verify permissions are checked (error due to function not callable)
+SET ROLE bttest_role;
+SELECT bt_index_check('bttest_a_idx'::regclass);
+ERROR:  permission denied for function bt_index_check
+SELECT bt_index_parent_check('bttest_a_idx'::regclass);
+ERROR:  permission denied for function bt_index_parent_check
+RESET ROLE;
+-- we, intentionally, don't check relation permissions - it's useful
+-- to run this cluster-wide with a restricted account, and as tested
+-- above explicit permission has to be granted for that.
+GRANT EXECUTE ON FUNCTION bt_index_check(regclass) TO bttest_role;
+GRANT EXECUTE ON FUNCTION bt_index_parentcheck(regclass) TO bttest_role;
+ERROR:  function bt_index_parentcheck(regclass) does not exist
+SET ROLE bttest_role;
+SELECT bt_index_check('bttest_a_idx');
+ bt_index_check 
+----------------
+ 
+(1 row)
+
+SELECT bt_index_parent_check('bttest_a_idx');
+ERROR:  permission denied for function bt_index_parent_check
+RESET ROLE;
+-- verify plain tables are rejected (error)
+SELECT bt_index_check('bttest_a');
+ERROR:  "bttest_a" is not an index
+SELECT bt_index_parent_check('bttest_a');
+ERROR:  "bttest_a" is not an index
+-- verify non-existing indexes are rejected (error)
+SELECT bt_index_check(17);
+ERROR:  could not open relation with OID 17
+SELECT bt_index_parent_check(17);
+ERROR:  could not open relation with OID 17
+-- verify wrong index types are rejected (error)
+BEGIN;
+CREATE INDEX bttest_a_brin_idx ON bttest_a USING brin(id);
+SELECT bt_index_parent_check('bttest_a_brin_idx');
+ERROR:  only B-Tree indexes are supported as targets for verification
+DETAIL:  Relation "bttest_a_brin_idx" is not a B-Tree index.
+ROLLBACK;
+-- normal check outside of xact
+SELECT bt_index_check('bttest_a_idx');
+ bt_index_check 
+----------------
+ 
+(1 row)
+
+-- more expansive test
+SELECT bt_index_parent_check('bttest_b_idx');
+ bt_index_parent_check 
+-----------------------
+ 
+(1 row)
+
+BEGIN;
+SELECT bt_index_check('bttest_a_idx');
+ bt_index_check 
+----------------
+ 
+(1 row)
+
+SELECT bt_index_parent_check('bttest_b_idx');
+ bt_index_parent_check 
+-----------------------
+ 
+(1 row)
+
+-- make sure we don't have any leftover locks
+SELECT * FROM pg_locks WHERE relation IN ('bttest_a_idx'::regclass, 'bttest_b_idx'::regclass);
+ locktype | database | relation | page | tuple | virtualxid | transactionid | classid | objid | objsubid | virtualtransaction | pid | mode | granted | fastpath 
+----------+----------+----------+------+-------+------------+---------------+---------+-------+----------+--------------------+-----+------+---------+----------
+(0 rows)
+
+COMMIT;
+-- cleanup
+DROP TABLE bttest_a;
+DROP TABLE bttest_b;
+DROP OWNED BY bttest_role; -- permissions
+DROP ROLE bttest_role;
diff --git a/contrib/amcheck/sql/check.sql b/contrib/amcheck/sql/check.sql
new file mode 100644
index 0000000000..5b3e6d530c
--- /dev/null
+++ b/contrib/amcheck/sql/check.sql
@@ -0,0 +1 @@
+CREATE EXTENSION amcheck;
diff --git a/contrib/amcheck/sql/check_btree.sql b/contrib/amcheck/sql/check_btree.sql
new file mode 100644
index 0000000000..53377f6da9
--- /dev/null
+++ b/contrib/amcheck/sql/check_btree.sql
@@ -0,0 +1,59 @@
+-- minimal test, basically just verifying that amcheck
+CREATE TABLE bttest_a(id int8);
+CREATE TABLE bttest_b(id int8);
+
+INSERT INTO bttest_a SELECT * FROM generate_series(1, 100000);
+INSERT INTO bttest_b SELECT * FROM generate_series(100000, 1, -1);
+
+CREATE INDEX bttest_a_idx ON bttest_a USING btree (id);
+CREATE INDEX bttest_b_idx ON bttest_b USING btree (id);
+
+CREATE ROLE bttest_role;
+
+-- verify permissions are checked (error due to function not callable)
+SET ROLE bttest_role;
+SELECT bt_index_check('bttest_a_idx'::regclass);
+SELECT bt_index_parent_check('bttest_a_idx'::regclass);
+RESET ROLE;
+
+-- we, intentionally, don't check relation permissions - it's useful
+-- to run this cluster-wide with a restricted account, and as tested
+-- above explicit permission has to be granted for that.
+GRANT EXECUTE ON FUNCTION bt_index_check(regclass) TO bttest_role;
+GRANT EXECUTE ON FUNCTION bt_index_parentcheck(regclass) TO bttest_role;
+SET ROLE bttest_role;
+SELECT bt_index_check('bttest_a_idx');
+SELECT bt_index_parent_check('bttest_a_idx');
+RESET ROLE;
+
+-- verify plain tables are rejected (error)
+SELECT bt_index_check('bttest_a');
+SELECT bt_index_parent_check('bttest_a');
+
+-- verify non-existing indexes are rejected (error)
+SELECT bt_index_check(17);
+SELECT bt_index_parent_check(17);
+
+-- verify wrong index types are rejected (error)
+BEGIN;
+CREATE INDEX bttest_a_brin_idx ON bttest_a USING brin(id);
+SELECT bt_index_parent_check('bttest_a_brin_idx');
+ROLLBACK;
+
+-- normal check outside of xact
+SELECT bt_index_check('bttest_a_idx');
+-- more expansive test
+SELECT bt_index_parent_check('bttest_b_idx');
+
+BEGIN;
+SELECT bt_index_check('bttest_a_idx');
+SELECT bt_index_parent_check('bttest_b_idx');
+-- make sure we don't have any leftover locks
+SELECT * FROM pg_locks WHERE relation IN ('bttest_a_idx'::regclass, 'bttest_b_idx'::regclass);
+COMMIT;
+
+-- cleanup
+DROP TABLE bttest_a;
+DROP TABLE bttest_b;
+DROP OWNED BY bttest_role; -- permissions
+DROP ROLE bttest_role;
diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
index 4cef4c44ff..ad8e15ea67 100644
--- a/contrib/amcheck/verify_nbtree.c
+++ b/contrib/amcheck/verify_nbtree.c
@@ -43,8 +43,8 @@ PG_MODULE_MAGIC;
  * target is the point of reference for a verification operation.
  *
  * Other B-Tree pages may be allocated, but those are always auxiliary (e.g.,
- * they are current target's child pages).  Conceptually, problems are only
- * ever found in the current target page.	Each page found by verification's
+ * they are current target's child pages). Conceptually, problems are only
+ * ever found in the current target page. Each page found by verification's
  * left/right, top/bottom scan becomes the target exactly once.
  */
 typedef struct BtreeCheckState
@@ -92,6 +92,7 @@ typedef struct BtreeLevel
 PG_FUNCTION_INFO_V1(bt_index_check);
 PG_FUNCTION_INFO_V1(bt_index_parent_check);
 
+static void bt_index_check_internal(Oid indrelid, bool parentcheck);
 static inline void btree_index_checkable(Relation rel);
 static void bt_check_every_level(Relation rel, bool exclusivelylocked);
 static BtreeLevel bt_check_level_from_leftmost(BtreeCheckState *state,
@@ -119,25 +120,15 @@ static Page palloc_btree_page(BtreeCheckState *state, BlockNumber blocknum);
  *
  * Verify integrity of B-Tree index.
  *
- * Only acquires AccessShareLock on index relation.  Does not consider
+ * Acquires AccessShareLock on heap & index relations.  Does not consider
  * invariants that exist between parent/child pages.
  */
 Datum
 bt_index_check(PG_FUNCTION_ARGS)
 {
-	Oid			relid = PG_GETARG_OID(0);
-	Relation	indrel;
+	Oid			indrelid = PG_GETARG_OID(0);
 
-	indrel = relation_open(relid, AccessShareLock);
-
-	/* Relation suitable for checking as B-Tree? */
-	btree_index_checkable(indrel);
-
-	/* Check index */
-	bt_check_every_level(indrel, false);
-
-	/* Close, but keep lock */
-	relation_close(indrel, NoLock);
+	bt_index_check_internal(indrelid, false);
 
 	PG_RETURN_VOID();
 }
@@ -147,36 +138,55 @@ bt_index_check(PG_FUNCTION_ARGS)
  *
  * Verify integrity of B-Tree index.
  *
- * Acquires ExclusiveLock on index relation, and ShareLock on the associated
- * heap relation, a bit like REINDEX.  Verifies that downlinks in parent pages
- * are valid lower bounds on child pages.
+ * Acquires ShareLock on heap & index relations.  Verifies that downlinks in
+ * parent pages are valid lower bounds on child pages.
  */
 Datum
 bt_index_parent_check(PG_FUNCTION_ARGS)
 {
 	Oid			indrelid = PG_GETARG_OID(0);
+
+	bt_index_check_internal(indrelid, true);
+
+	PG_RETURN_VOID();
+}
+
+/*
+ * Helper for bt_index_[parent_]check, coordinating the bulk of the work.
+ */
+static void
+bt_index_check_internal(Oid indrelid, bool parentcheck)
+{
 	Oid			heapid;
 	Relation	indrel;
 	Relation	heaprel;
+	LOCKMODE	lockmode;
+
+	if (parentcheck)
+		lockmode = ShareLock;
+	else
+		lockmode = AccessShareLock;
 
 	/*
-	 * We must lock table before index to avoid deadlocks.	However, if the
+	 * We must lock table before index to avoid deadlocks.  However, if the
 	 * passed indrelid isn't an index then IndexGetRelation() will fail.
 	 * Rather than emitting a not-very-helpful error message, postpone
 	 * complaining, expecting that the is-it-an-index test below will fail.
+	 *
+	 * In hot standby mode this will raise an error when parentcheck is true.
 	 */
 	heapid = IndexGetRelation(indrelid, true);
 	if (OidIsValid(heapid))
-		heaprel = heap_open(heapid, ShareLock);
+		heaprel = heap_open(heapid, lockmode);
 	else
 		heaprel = NULL;
 
 	/*
 	 * Open the target index relations separately (like relation_openrv(), but
 	 * with heap relation locked first to prevent deadlocking).  In hot
-	 * standby mode this will raise an error.
+	 * standby mode this will raise an error when parentcheck is true.
 	 */
-	indrel = index_open(indrelid, ExclusiveLock);
+	indrel = index_open(indrelid, lockmode);
 
 	/*
 	 * Since we did the IndexGetRelation call above without any lock, it's
@@ -195,19 +205,25 @@ bt_index_parent_check(PG_FUNCTION_ARGS)
 	btree_index_checkable(indrel);
 
 	/* Check index */
-	bt_check_every_level(indrel, true);
+	bt_check_every_level(indrel, parentcheck);
 
-	/* Close, but keep locks */
-	index_close(indrel, NoLock);
+	/*
+	 * Release locks early. That's ok here because nothing in the called
+	 * routines will trigger shared cache invalidations to be sent, so we can
+	 * relax the usual pattern of only releasing locks after commit.
+	 */
+	index_close(indrel, lockmode);
 	if (heaprel)
-		heap_close(heaprel, NoLock);
-
-	PG_RETURN_VOID();
+		heap_close(heaprel, lockmode);
 }
 
 /*
  * Basic checks about the suitability of a relation for checking as a B-Tree
  * index.
+ *
+ * NB: Intentionally not checking permissions, the function is normally not
+ * callable by non-superusers. If granted, it's useful to be able to check a
+ * whole cluster.
  */
 static inline void
 btree_index_checkable(Relation rel)
@@ -236,7 +252,7 @@ btree_index_checkable(Relation rel)
 }
 
 /*
- * Main entry point for B-Tree SQL-callable functions.	Walks the B-Tree in
+ * Main entry point for B-Tree SQL-callable functions. Walks the B-Tree in
  * logical order, verifying invariants as it goes.
  *
  * It is the caller's responsibility to acquire appropriate heavyweight lock on
@@ -265,7 +281,7 @@ bt_check_every_level(Relation rel, bool exclusivelylocked)
 	BtreeLevel	current;
 
 	/*
-	 * RecentGlobalXmin assertion matches index_getnext_tid().	See note on
+	 * RecentGlobalXmin assertion matches index_getnext_tid().  See note on
 	 * RecentGlobalXmin/B-Tree page deletion.
 	 */
 	Assert(TransactionIdIsValid(RecentGlobalXmin));
@@ -301,15 +317,13 @@ bt_check_every_level(Relation rel, bool exclusivelylocked)
 				(errcode(ERRCODE_NO_DATA),
 				 errmsg("harmless fast root mismatch in index %s",
 						RelationGetRelationName(rel)),
-				 errdetail_internal("Fast block %u (level %u) "
-									"differs from true root "
-									"block %u (level %u).",
+				 errdetail_internal("Fast block %u (level %u) differs from true root block %u (level %u).",
 									metad->btm_fastroot, metad->btm_fastlevel,
 									metad->btm_root, metad->btm_level)));
 
 	/*
 	 * Starting at the root, verify every level.  Move left to right, top to
-	 * bottom.	Note that there may be no pages other than the meta page (meta
+	 * bottom.  Note that there may be no pages other than the meta page (meta
 	 * page can indicate that root is P_NONE when the index is totally empty).
 	 */
 	previouslevel = InvalidBtreeLevel;
@@ -346,9 +360,9 @@ bt_check_every_level(Relation rel, bool exclusivelylocked)
  *
  * Returns state for next call, if any.  This includes left-most block number
  * one level lower that should be passed on next level/call, which is set to
- * P_NONE on last call here (when leaf level is verified).	Level numbers
+ * P_NONE on last call here (when leaf level is verified).  Level numbers
  * follow the nbtree convention: higher levels have higher numbers, because new
- * levels are added only due to a root page split.	Note that prior to the
+ * levels are added only due to a root page split.  Note that prior to the
  * first root page split, the root is also a leaf page, so there is always a
  * level 0 (leaf level), and it's always the last level processed.
  *
@@ -610,8 +624,7 @@ bt_target_page_check(BtreeCheckState *state)
 					(errcode(ERRCODE_INDEX_CORRUPTED),
 					 errmsg("high key invariant violated for index \"%s\"",
 							RelationGetRelationName(state->rel)),
-					 errdetail_internal("Index tid=%s points to %s tid=%s "
-										"page lsn=%X/%X.",
+					 errdetail_internal("Index tid=%s points to %s tid=%s page lsn=%X/%X.",
 										itid,
 										P_ISLEAF(topaque) ? "heap" : "index",
 										htid,
@@ -676,7 +689,7 @@ bt_target_page_check(BtreeCheckState *state)
 		 * already been performed for every other "real" item on target page
 		 * when last item is checked.  The difference is that the next item
 		 * (the item that is compared to target's last item) needs to come
-		 * from the next/sibling page.	There may not be such an item
+		 * from the next/sibling page.  There may not be such an item
 		 * available from sibling for various reasons, though (e.g., target is
 		 * the rightmost page on level).
 		 */
@@ -716,8 +729,7 @@ bt_target_page_check(BtreeCheckState *state)
 						(errcode(ERRCODE_INDEX_CORRUPTED),
 						 errmsg("cross page item order invariant violated for index \"%s\"",
 								RelationGetRelationName(state->rel)),
-						 errdetail_internal("Last item on page tid=(%u,%u) "
-											"page lsn=%X/%X.",
+						 errdetail_internal("Last item on page tid=(%u,%u) page lsn=%X/%X.",
 											state->targetblock, offset,
 											(uint32) (state->targetlsn >> 32),
 											(uint32) state->targetlsn)));
@@ -787,7 +799,7 @@ bt_right_page_check_scankey(BtreeCheckState *state)
 	 * (or left link, or downlink) and find its sibling, because recycling
 	 * does not occur until no possible index scan could land on the page.
 	 * Index scans can follow links with nothing more than their snapshot as
-	 * an interlock and be sure of at least that much.	(See page
+	 * an interlock and be sure of at least that much.  (See page
 	 * recycling/RecentGlobalXmin notes in nbtree README.)
 	 *
 	 * Furthermore, it's okay if we follow a rightlink and find a half-dead or
@@ -860,7 +872,7 @@ bt_right_page_check_scankey(BtreeCheckState *state)
 	 * deleted) implies that target page was not merged into from the right by
 	 * deletion; the key space at or after target never moved left.  Target's
 	 * parent either has the same downlink to target as before, or a <=
-	 * downlink due to deletion at the left of target.	Target either has the
+	 * downlink due to deletion at the left of target.  Target either has the
 	 * same highkey as before, or a highkey <= before when there is a page
 	 * split. (The rightmost concurrently-split-from-target-page page will
 	 * still have the same highkey as target was originally found to have,
@@ -907,7 +919,7 @@ bt_right_page_check_scankey(BtreeCheckState *state)
 	 * that happens only to determine if there was a concurrent page deletion
 	 * of a reference page, and just like us if there was a page deletion of
 	 * that reference page it means we can move on from caring about the
-	 * reference page.	See the nbtree README for a full description of how
+	 * reference page.  See the nbtree README for a full description of how
 	 * that works.
 	 */
 	nline = PageGetMaxOffsetNumber(rightpage);
@@ -946,7 +958,7 @@ bt_right_page_check_scankey(BtreeCheckState *state)
 	}
 
 	/*
-	 * Return first real item scankey.	Note that this relies on right page
+	 * Return first real item scankey.  Note that this relies on right page
 	 * memory remaining allocated.
 	 */
 	return _bt_mkscankey(state->rel,
@@ -956,7 +968,7 @@ bt_right_page_check_scankey(BtreeCheckState *state)
 /*
  * Checks one of target's downlink against its child page.
  *
- * Conceptually, the target page continues to be what is checked here.	The
+ * Conceptually, the target page continues to be what is checked here.  The
  * target block is still blamed in the event of finding an invariant violation.
  * The downlink insertion into the target is probably where any problem raised
  * here arises, and there is no such thing as a parent link, so doing the
@@ -977,8 +989,8 @@ bt_downlink_check(BtreeCheckState *state, BlockNumber childblock,
 	 *
 	 * NB: In general, page deletion deletes the right sibling's downlink, not
 	 * the downlink of the page being deleted; the deleted page's downlink is
-	 * reused for its sibling.	The key space is thereby consolidated between
-	 * the deleted page and its right sibling.	(We cannot delete a parent
+	 * reused for its sibling.  The key space is thereby consolidated between
+	 * the deleted page and its right sibling.  (We cannot delete a parent
 	 * page's rightmost child unless it is the last child page, and we intend
 	 * to also delete the parent itself.)
 	 *
@@ -1041,11 +1053,10 @@ bt_downlink_check(BtreeCheckState *state, BlockNumber childblock,
 					(errcode(ERRCODE_INDEX_CORRUPTED),
 					 errmsg("down-link lower bound invariant violated for index \"%s\"",
 							RelationGetRelationName(state->rel)),
-				errdetail_internal("Parent block=%u child index tid=(%u,%u) "
-								   "parent page lsn=%X/%X.",
-								   state->targetblock, childblock, offset,
-								   (uint32) (state->targetlsn >> 32),
-								   (uint32) state->targetlsn)));
+					 errdetail_internal("Parent block=%u child index tid=(%u,%u) parent page lsn=%X/%X.",
+									  state->targetblock, childblock, offset,
+										(uint32) (state->targetlsn >> 32),
+										(uint32) state->targetlsn)));
 	}
 
 	pfree(child);
@@ -1066,7 +1077,7 @@ offset_is_negative_infinity(BTPageOpaque opaque, OffsetNumber offset)
 {
 	/*
 	 * For internal pages only, the first item after high key, if any, is
-	 * negative infinity item.	Internal pages always have a negative infinity
+	 * negative infinity item.  Internal pages always have a negative infinity
 	 * item, whereas leaf pages never have one.  This implies that negative
 	 * infinity item is either first or second line item, or there is none
 	 * within page.
diff --git a/doc/src/sgml/amcheck.sgml b/doc/src/sgml/amcheck.sgml
index 168d3f4397..893a5b41d9 100644
--- a/doc/src/sgml/amcheck.sgml
+++ b/doc/src/sgml/amcheck.sgml
@@ -24,7 +24,7 @@
   collated lexical order).  If that particular invariant somehow fails
   to hold, we can expect binary searches on the affected page to
   incorrectly guide index scans, resulting in wrong answers to SQL
-  queries.  Problems like this can be very subtle.
+  queries.
  </para>
  <para>
   Verification is performed using the same procedures as those used by
@@ -90,9 +90,9 @@ ORDER BY c.relpages DESC LIMIT 10;
       database where verification is supported.
      </para>
      <para>
-      An <literal>AccessShareLock</> is acquired on the target index
-      by <function>bt_index_check</function>.  This lock mode is the
-      same lock mode acquired on relations by simple
+      <function>bt_index_check</function> acquires an <literal>AccessShareLock</>
+      on the target index and the heap relation it belongs to. This lock mode
+      is the same lock mode acquired on relations by simple
       <literal>SELECT</> statements.
       <function>bt_index_check</function> does not verify invariants
       that span child/parent relationships, nor does it verify that
@@ -132,7 +132,7 @@ ORDER BY c.relpages DESC LIMIT 10;
       logical inconsistency or other problem.
      </para>
      <para>
-      An <literal>ExclusiveLock</> is required on the target index by
+      A <literal>ShareLock</> is required on the target index by
       <function>bt_index_parent_check</function> (a
       <literal>ShareLock</> is also acquired on the heap relation).
       These locks prevent concurrent data modification from
@@ -140,9 +140,7 @@ ORDER BY c.relpages DESC LIMIT 10;
       commands.  The locks also prevent the underlying relation from
       being concurrently processed by <command>VACUUM</>, as well as
       all other utility commands.  Note that the function holds locks
-      for the entire duration of the transaction, so there can be a
-      significant advantage to verifying each index individually, each
-      in its own dedicated transaction.
+      only while running, not for the entire transaction.
      </para>
      <para>
       <function>bt_index_parent_check</function>'s additional
-- 
2.11.0.22.g8d7a455.dirty

