diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 84becb9f3f..743d5ea61a 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -40,7 +40,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/partition.h" @@ -4700,12 +4699,12 @@ RelationGetIndexPredicate(Relation relation) * 2. "recheck_on_update" index option explicitly set by user, which overrides 1) */ static bool -IsProjectionFunctionalIndex(Relation index, IndexInfo *ii) +IsProjectionFunctionalIndex(Relation index) { bool is_projection = false; #ifdef NOT_USED - if (ii->ii_Expressions) + if (RelationGetIndexExpressions(index)) { HeapTuple tuple; Datum reloptions; @@ -4715,7 +4714,7 @@ IsProjectionFunctionalIndex(Relation index, IndexInfo *ii) /* by default functional index is considered as non-injective */ is_projection = true; - cost_qual_eval(&index_expr_cost, ii->ii_Expressions, NULL); + cost_qual_eval(&index_expr_cost, RelationGetIndexExpressions(index), NULL); /* * If index expression is too expensive, then disable projection @@ -4861,7 +4860,10 @@ 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 isPK; /* primary key */ @@ -4869,13 +4871,33 @@ restart: 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 a primary key? */ isPK = (indexOid == relpkindex); @@ -4884,9 +4906,9 @@ restart: 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_IndexAttrNumbers[i]; + int attrnum = indexDesc->rd_index->indkey.values[i]; /* * Since we have covering indexes with non-key columns, we must @@ -4901,33 +4923,33 @@ restart: indexattrs = bms_add_member(indexattrs, attrnum - FirstLowInvalidHeapAttributeNumber); - if (isKey && i < indexInfo->ii_NumIndexKeyAttrs) + if (isKey && i < indexDesc->rd_index->indnkeyatts) uindexattrs = bms_add_member(uindexattrs, attrnum - FirstLowInvalidHeapAttributeNumber); - if (isPK && i < indexInfo->ii_NumIndexKeyAttrs) + if (isPK && i < indexDesc->rd_index->indnkeyatts) pkindexattrs = bms_add_member(pkindexattrs, attrnum - FirstLowInvalidHeapAttributeNumber); - if (isIDKey && i < indexInfo->ii_NumIndexKeyAttrs) + if (isIDKey && i < indexDesc->rd_index->indnkeyatts) idindexattrs = bms_add_member(idindexattrs, attrnum - FirstLowInvalidHeapAttributeNumber); } } /* Collect attributes used in expressions, too */ - if (IsProjectionFunctionalIndex(indexDesc, indexInfo)) + if (IsProjectionFunctionalIndex(indexDesc)) { projindexes = bms_add_member(projindexes, indexno); - pull_varattnos((Node *) indexInfo->ii_Expressions, 1, &projindexattrs); + pull_varattnos(indexExpressions, 1, &projindexattrs); } else { /* 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); indexno += 1; diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl new file mode 100644 index 0000000000..cbdb30a8bb --- /dev/null +++ b/src/test/subscription/t/100_bugs.pl @@ -0,0 +1,65 @@ +# Tests for various bugs found over time +use strict; +use warnings; +use PostgresNode; +use TestLib; +use Test::More tests => 1; + +# Bug #15114 + +# 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. The +# fix was to avoid the constant expressions simplification in +# RelationGetIndexAttrBitmap(), so it's safe to call in more contexts. + +my $node_publisher = get_new_node('publisher'); +$node_publisher->init(allows_streaming => 'logical'); +$node_publisher->start; + +my $node_subscriber = get_new_node('subscriber'); +$node_subscriber->init(allows_streaming => 'logical'); +$node_subscriber->start; + +my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres'; + +$node_publisher->safe_psql('postgres', + "CREATE TABLE tab1 (a int PRIMARY KEY, b int)"); + +$node_publisher->safe_psql('postgres', + "CREATE FUNCTION double(x int) RETURNS int IMMUTABLE LANGUAGE SQL AS 'select x * 2'"); + +# an index with a predicate that lends itself to constant expressions +# evaluation +$node_publisher->safe_psql('postgres', + "CREATE INDEX ON tab1 (b) WHERE a > double(1)"); + +# and the same setup on the subscriber +$node_subscriber->safe_psql('postgres', + "CREATE TABLE tab1 (a int PRIMARY KEY, b int)"); + +$node_subscriber->safe_psql('postgres', + "CREATE FUNCTION double(x int) RETURNS int IMMUTABLE LANGUAGE SQL AS 'select x * 2'"); + +$node_subscriber->safe_psql('postgres', + "CREATE INDEX ON tab1 (b) WHERE a > double(1)"); + +$node_publisher->safe_psql('postgres', + "CREATE PUBLICATION pub1 FOR ALL TABLES"); + +$node_subscriber->safe_psql('postgres', + "CREATE SUBSCRIPTION sub1 CONNECTION '$publisher_connstr application_name=sub1' PUBLICATION pub1"); + +$node_publisher->wait_for_catchup('sub1'); + +# This would crash, first on the publisher, and then (if the publisher +# is fixed) on the subscriber. +$node_publisher->safe_psql('postgres', + "INSERT INTO tab1 VALUES (1, 2)"); + +$node_publisher->wait_for_catchup('sub1'); + +pass('index predicates do not cause crash');