diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index d5a1017985c..e2be8ea28c8 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -38,7 +38,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" @@ -4948,7 +4947,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 */ @@ -4956,13 +4958,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); @@ -4971,9 +4993,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_KeyAttrNumbers[i]; + int attrnum = indexDesc->rd_index->indkey.values[i]; if (attrnum != 0) { @@ -4995,10 +5017,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); } diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl new file mode 100644 index 00000000000..35171c01466 --- /dev/null +++ b/src/test/subscription/t/100_bugs.pl @@ -0,0 +1,74 @@ +# Tests for various bugs found over time +use strict; +use warnings; +use PostgresNode; +use TestLib; +use Test::More tests => 1; + +sub wait_for_caught_up +{ + my ($node, $appname) = @_; + + $node->poll_query_until('postgres', +"SELECT pg_current_wal_lsn() <= replay_lsn FROM pg_stat_replication WHERE application_name = '$appname';" + ) or die "Timed out while waiting for subscriber to catch up"; +} + +# 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"); + +wait_for_caught_up($node_publisher, '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)"); + +wait_for_caught_up($node_publisher, 'sub1'); + +pass('index predicates do not cause crash');