From 650296f8f607365278f64c9b9a7438c321f2d4ec Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Mon, 28 Jan 2019 22:09:33 +0100 Subject: [PATCH] Fix a crash in logical replication MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The bug was that determining which columns are part of the replica identity index using RelationGetIndexAttrBitmap() would run eval_const_expressions() on index expressions and predicates across all indexes of the table, which in turn might require a snapshot, but there wasn't one set, so it crashes. There were actually two separate bugs, one on the publisher and one on the subscriber. To trigger the bug, a table that is part of a publication or subscription needs to have an index with a predicate or expression that lends itself to constant expressions simplification. The fix is to avoid the constant expressions simplification in RelationGetIndexAttrBitmap(), so that it becomes safe to call in these contexts. The constant expressions simplification comes from the calls to RelationGetIndexExpressions()/RelationGetIndexPredicate() via BuildIndexInfo(). But RelationGetIndexAttrBitmap() calling BuildIndexInfo() is overkill. The latter just takes pg_index catalog information, packs it into the IndexInfo structure, which former then just unpacks again and throws away. We can just do this directly with less overhead and skip the troublesome calls to eval_const_expressions(). This also removes the awkward cross-dependency between relcache.c and index.c. Bug: #15114 Reported-by: Петър Славов Reviewed-by: Noah Misch Reviewed-by: Michael Paquier Discussion: https://www.postgresql.org/message-id/flat/152110589574.1223.17983600132321618383@wrigleys.postgresql.org/ --- src/backend/utils/cache/relcache.c | 44 ++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 0853a0c913..7b1b3875fa 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -37,7 +37,6 @@ #include "access/xact.h" #include "access/xlog.h" #include "catalog/catalog.h" -#include "catalog/index.h" #include "catalog/indexing.h" #include "catalog/namespace.h" #include "catalog/pg_am.h" @@ -4510,28 +4509,51 @@ restart: { Oid indexOid = lfirst_oid(l); Relation indexDesc; - IndexInfo *indexInfo; + Datum datum; + bool isnull; + Node *indexExpressions; + Node *indexPredicate; int i; bool isKey; /* candidate key */ bool isIDKey; /* replica identity index */ indexDesc = index_open(indexOid, AccessShareLock); - /* Extract index key information from the index's pg_index row */ - indexInfo = BuildIndexInfo(indexDesc); + /* + * Extract index expressions and index predicate. Note: Don't use + * RelationGetIndexExpressions()/RelationGetIndexPredicate(), because + * those might run constant expressions evaluation, which needs a + * snapshot, which we might not have here. (Also, it's probably more + * sound to collect the bitmaps before any transformations that might + * eliminate columns, but the practical impact of this is limited.) + */ + + datum = heap_getattr(indexDesc->rd_indextuple, Anum_pg_index_indexprs, + GetPgIndexDescriptor(), &isnull); + if (!isnull) + indexExpressions = stringToNode(TextDatumGetCString(datum)); + else + indexExpressions = NULL; + + datum = heap_getattr(indexDesc->rd_indextuple, Anum_pg_index_indpred, + GetPgIndexDescriptor(), &isnull); + if (!isnull) + indexPredicate = stringToNode(TextDatumGetCString(datum)); + else + indexPredicate = NULL; /* Can this index be referenced by a foreign key? */ - isKey = indexInfo->ii_Unique && - indexInfo->ii_Expressions == NIL && - indexInfo->ii_Predicate == NIL; + isKey = indexDesc->rd_index->indisunique && + indexExpressions == NULL && + indexPredicate == NULL; /* Is this index the configured (or default) replica identity? */ isIDKey = (indexOid == relreplindex); /* Collect simple attribute references */ - for (i = 0; i < indexInfo->ii_NumIndexAttrs; i++) + for (i = 0; i < indexDesc->rd_index->indnatts; i++) { - int attrnum = indexInfo->ii_KeyAttrNumbers[i]; + int attrnum = indexDesc->rd_index->indkey.values[i]; if (attrnum != 0) { @@ -4549,10 +4571,10 @@ restart: } /* Collect all attributes used in expressions, too */ - pull_varattnos((Node *) indexInfo->ii_Expressions, 1, &indexattrs); + pull_varattnos(indexExpressions, 1, &indexattrs); /* Collect all attributes in the index predicate, too */ - pull_varattnos((Node *) indexInfo->ii_Predicate, 1, &indexattrs); + pull_varattnos(indexPredicate, 1, &indexattrs); index_close(indexDesc, AccessShareLock); } -- 2.40.0