invalidate_indexattr_v3.patch

text/plain

Filename: invalidate_indexattr_v3.patch
Type: text/plain
Part: 0
Message: Re: Index corruption with CREATE INDEX CONCURRENTLY

Patch

Format: unified
Series: patch v3
File+
src/backend/utils/cache/relcache.c 23 15
commit 7054c21883154f67058d42180606e0c5428d2745
Author:     Alvaro Herrera <alvherre@alvh.no-ip.org>
AuthorDate: Fri Feb 3 15:40:37 2017 -0300
CommitDate: Fri Feb 3 15:41:59 2017 -0300

    Fix CREATE INDEX CONCURRENTLY relcache bug

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 8a7c560..003ccfa 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -4745,9 +4745,11 @@ RelationGetIndexPredicate(Relation relation)
  * Attribute numbers are offset by FirstLowInvalidHeapAttributeNumber so that
  * we can include system attributes (e.g., OID) in the bitmap representation.
  *
- * Caller had better hold at least RowExclusiveLock on the target relation
- * to ensure that it has a stable set of indexes.  This also makes it safe
- * (deadlock-free) for us to take locks on the relation's indexes.
+ * While all callers should at least RowExclusiveLock on the target relation,
+ * we still can't guarantee a stable set of indexes because CREATE INDEX
+ * CONCURRENTLY and DROP INDEX CONCURRENTLY can change set of indexes, without
+ * taking any conflicting lock. So we must be prepared to handle changes in the
+ * set of indexes and recompute bitmaps, when necessary.
  *
  * The returned result is palloc'd in the caller's memory context and should
  * be bms_free'd when not needed anymore.
@@ -4763,7 +4765,6 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
 	Oid			relpkindex;
 	Oid			relreplindex;
 	ListCell   *l;
-	MemoryContext oldcxt;
 
 	/* Quick exit if we already computed the result. */
 	if (relation->rd_indexattr != NULL)
@@ -4807,6 +4808,8 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
 	relpkindex = relation->rd_pkindex;
 	relreplindex = relation->rd_replidindex;
 
+	Assert(relation->rd_indexvalid != 0);
+
 	/*
 	 * For each index, add referenced attributes to indexattrs.
 	 *
@@ -4893,18 +4896,23 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
 	relation->rd_idattr = NULL;
 
 	/*
-	 * Now save copies of the bitmaps in the relcache entry.  We intentionally
-	 * set rd_indexattr last, because that's the one that signals validity of
-	 * the values; if we run out of memory before making that copy, we won't
-	 * leave the relcache entry looking like the other ones are valid but
-	 * empty.
+	 * Now save copies of the bitmaps in the relcache entry, but only if no
+	 * invalidation occured in the meantime.  We intentionally set rd_indexattr
+	 * last, because that's the one that signals validity of the values; if we
+	 * run out of memory before making that copy, we won't leave the relcache
+	 * entry looking like the other ones are valid but empty.
 	 */
-	oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
-	relation->rd_keyattr = bms_copy(uindexattrs);
-	relation->rd_pkattr = bms_copy(pkindexattrs);
-	relation->rd_idattr = bms_copy(idindexattrs);
-	relation->rd_indexattr = bms_copy(indexattrs);
-	MemoryContextSwitchTo(oldcxt);
+	if (relation->rd_indexvalid != 0)
+	{
+		MemoryContext oldcxt;
+
+		oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
+		relation->rd_keyattr = bms_copy(uindexattrs);
+		relation->rd_pkattr = bms_copy(pkindexattrs);
+		relation->rd_idattr = bms_copy(idindexattrs);
+		relation->rd_indexattr = bms_copy(indexattrs);
+		MemoryContextSwitchTo(oldcxt);
+	}
 
 	/* We return our original working copy for caller to play with */
 	switch (attrKind)