Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 650296f

Browse files
committed
Fix a crash in logical replication
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: Петър Славов <pet.slavov@gmail.com> Reviewed-by: Noah Misch <noah@leadboat.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/152110589574.1223.17983600132321618383@wrigleys.postgresql.org/
1 parent ae4c7d5 commit 650296f

File tree

1 file changed

+33
-11
lines changed

1 file changed

+33
-11
lines changed

src/backend/utils/cache/relcache.c

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
#include "access/xact.h"
3838
#include "access/xlog.h"
3939
#include "catalog/catalog.h"
40-
#include "catalog/index.h"
4140
#include "catalog/indexing.h"
4241
#include "catalog/namespace.h"
4342
#include "catalog/pg_am.h"
@@ -4510,28 +4509,51 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
45104509
{
45114510
Oid indexOid = lfirst_oid(l);
45124511
Relation indexDesc;
4513-
IndexInfo *indexInfo;
4512+
Datum datum;
4513+
bool isnull;
4514+
Node *indexExpressions;
4515+
Node *indexPredicate;
45144516
int i;
45154517
bool isKey; /* candidate key */
45164518
bool isIDKey; /* replica identity index */
45174519

45184520
indexDesc = index_open(indexOid, AccessShareLock);
45194521

4520-
/* Extract index key information from the index's pg_index row */
4521-
indexInfo = BuildIndexInfo(indexDesc);
4522+
/*
4523+
* Extract index expressions and index predicate. Note: Don't use
4524+
* RelationGetIndexExpressions()/RelationGetIndexPredicate(), because
4525+
* those might run constant expressions evaluation, which needs a
4526+
* snapshot, which we might not have here. (Also, it's probably more
4527+
* sound to collect the bitmaps before any transformations that might
4528+
* eliminate columns, but the practical impact of this is limited.)
4529+
*/
4530+
4531+
datum = heap_getattr(indexDesc->rd_indextuple, Anum_pg_index_indexprs,
4532+
GetPgIndexDescriptor(), &isnull);
4533+
if (!isnull)
4534+
indexExpressions = stringToNode(TextDatumGetCString(datum));
4535+
else
4536+
indexExpressions = NULL;
4537+
4538+
datum = heap_getattr(indexDesc->rd_indextuple, Anum_pg_index_indpred,
4539+
GetPgIndexDescriptor(), &isnull);
4540+
if (!isnull)
4541+
indexPredicate = stringToNode(TextDatumGetCString(datum));
4542+
else
4543+
indexPredicate = NULL;
45224544

45234545
/* Can this index be referenced by a foreign key? */
4524-
isKey = indexInfo->ii_Unique &&
4525-
indexInfo->ii_Expressions == NIL &&
4526-
indexInfo->ii_Predicate == NIL;
4546+
isKey = indexDesc->rd_index->indisunique &&
4547+
indexExpressions == NULL &&
4548+
indexPredicate == NULL;
45274549

45284550
/* Is this index the configured (or default) replica identity? */
45294551
isIDKey = (indexOid == relreplindex);
45304552

45314553
/* Collect simple attribute references */
4532-
for (i = 0; i < indexInfo->ii_NumIndexAttrs; i++)
4554+
for (i = 0; i < indexDesc->rd_index->indnatts; i++)
45334555
{
4534-
int attrnum = indexInfo->ii_KeyAttrNumbers[i];
4556+
int attrnum = indexDesc->rd_index->indkey.values[i];
45354557

45364558
if (attrnum != 0)
45374559
{
@@ -4549,10 +4571,10 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
45494571
}
45504572

45514573
/* Collect all attributes used in expressions, too */
4552-
pull_varattnos((Node *) indexInfo->ii_Expressions, 1, &indexattrs);
4574+
pull_varattnos(indexExpressions, 1, &indexattrs);
45534575

45544576
/* Collect all attributes in the index predicate, too */
4555-
pull_varattnos((Node *) indexInfo->ii_Predicate, 1, &indexattrs);
4577+
pull_varattnos(indexPredicate, 1, &indexattrs);
45564578

45574579
index_close(indexDesc, AccessShareLock);
45584580
}

0 commit comments

Comments
 (0)