commit 7054c21883154f67058d42180606e0c5428d2745 Author: Alvaro Herrera 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)