Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitdfa774f

Browse files
committed
Fix a crash in logical replication
The bug was that determining which columns are part of the replicaidentity index using RelationGetIndexAttrBitmap() would runeval_const_expressions() on index expressions and predicates acrossall indexes of the table, which in turn might require a snapshot, butthere wasn't one set, so it crashes. There were actually two separatebugs, one on the publisher and one on the subscriber.To trigger the bug, a table that is part of a publication orsubscription needs to have an index with a predicate or expressionthat lends itself to constant expressions simplification.The fix is to avoid the constant expressions simplification inRelationGetIndexAttrBitmap(), so that it becomes safe to call in thesecontexts. The constant expressions simplification comes from thecalls to RelationGetIndexExpressions()/RelationGetIndexPredicate() viaBuildIndexInfo(). But RelationGetIndexAttrBitmap() callingBuildIndexInfo() is overkill. The latter just takes pg_index cataloginformation, packs it into the IndexInfo structure, which former thenjust unpacks again and throws away. We can just do this directly withless overhead and skip the troublesome calls toeval_const_expressions(). This also removes the awkwardcross-dependency between relcache.c and index.c.Bug: #15114Reported-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 parentb8f73df commitdfa774f

File tree

2 files changed

+101
-14
lines changed

2 files changed

+101
-14
lines changed

‎src/backend/utils/cache/relcache.c

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
#include"access/xact.h"
4242
#include"access/xlog.h"
4343
#include"catalog/catalog.h"
44-
#include"catalog/index.h"
4544
#include"catalog/indexing.h"
4645
#include"catalog/namespace.h"
4746
#include"catalog/partition.h"
@@ -4716,21 +4715,44 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
47164715
{
47174716
OidindexOid=lfirst_oid(l);
47184717
RelationindexDesc;
4719-
IndexInfo*indexInfo;
4718+
Datumdatum;
4719+
boolisnull;
4720+
Node*indexExpressions;
4721+
Node*indexPredicate;
47204722
inti;
47214723
boolisKey;/* candidate key */
47224724
boolisPK;/* primary key */
47234725
boolisIDKey;/* replica identity index */
47244726

47254727
indexDesc=index_open(indexOid,AccessShareLock);
47264728

4727-
/* Extract index key information from the index's pg_index row */
4728-
indexInfo=BuildIndexInfo(indexDesc);
4729+
/*
4730+
* Extract index expressions and index predicate. Note: Don't use
4731+
* RelationGetIndexExpressions()/RelationGetIndexPredicate(), because
4732+
* those might run constant expressions evaluation, which needs a
4733+
* snapshot, which we might not have here. (Also, it's probably more
4734+
* sound to collect the bitmaps before any transformations that might
4735+
* eliminate columns, but the practical impact of this is limited.)
4736+
*/
4737+
4738+
datum=heap_getattr(indexDesc->rd_indextuple,Anum_pg_index_indexprs,
4739+
GetPgIndexDescriptor(),&isnull);
4740+
if (!isnull)
4741+
indexExpressions=stringToNode(TextDatumGetCString(datum));
4742+
else
4743+
indexExpressions=NULL;
4744+
4745+
datum=heap_getattr(indexDesc->rd_indextuple,Anum_pg_index_indpred,
4746+
GetPgIndexDescriptor(),&isnull);
4747+
if (!isnull)
4748+
indexPredicate=stringToNode(TextDatumGetCString(datum));
4749+
else
4750+
indexPredicate=NULL;
47294751

47304752
/* Can this index be referenced by a foreign key? */
4731-
isKey=indexInfo->ii_Unique&&
4732-
indexInfo->ii_Expressions==NIL&&
4733-
indexInfo->ii_Predicate==NIL;
4753+
isKey=indexDesc->rd_index->indisunique&&
4754+
indexExpressions==NULL&&
4755+
indexPredicate==NULL;
47344756

47354757
/* Is this a primary key? */
47364758
isPK= (indexOid==relpkindex);
@@ -4739,9 +4761,9 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
47394761
isIDKey= (indexOid==relreplindex);
47404762

47414763
/* Collect simple attribute references */
4742-
for (i=0;i<indexInfo->ii_NumIndexAttrs;i++)
4764+
for (i=0;i<indexDesc->rd_index->indnatts;i++)
47434765
{
4744-
intattrnum=indexInfo->ii_IndexAttrNumbers[i];
4766+
intattrnum=indexDesc->rd_index->indkey.values[i];
47454767

47464768
/*
47474769
* Since we have covering indexes with non-key columns, we must
@@ -4756,25 +4778,25 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
47564778
indexattrs=bms_add_member(indexattrs,
47574779
attrnum-FirstLowInvalidHeapAttributeNumber);
47584780

4759-
if (isKey&&i<indexInfo->ii_NumIndexKeyAttrs)
4781+
if (isKey&&i<indexDesc->rd_index->indnkeyatts)
47604782
uindexattrs=bms_add_member(uindexattrs,
47614783
attrnum-FirstLowInvalidHeapAttributeNumber);
47624784

4763-
if (isPK&&i<indexInfo->ii_NumIndexKeyAttrs)
4785+
if (isPK&&i<indexDesc->rd_index->indnkeyatts)
47644786
pkindexattrs=bms_add_member(pkindexattrs,
47654787
attrnum-FirstLowInvalidHeapAttributeNumber);
47664788

4767-
if (isIDKey&&i<indexInfo->ii_NumIndexKeyAttrs)
4789+
if (isIDKey&&i<indexDesc->rd_index->indnkeyatts)
47684790
idindexattrs=bms_add_member(idindexattrs,
47694791
attrnum-FirstLowInvalidHeapAttributeNumber);
47704792
}
47714793
}
47724794

47734795
/* Collect all attributes used in expressions, too */
4774-
pull_varattnos((Node*)indexInfo->ii_Expressions,1,&indexattrs);
4796+
pull_varattnos(indexExpressions,1,&indexattrs);
47754797

47764798
/* Collect all attributes in the index predicate, too */
4777-
pull_varattnos((Node*)indexInfo->ii_Predicate,1,&indexattrs);
4799+
pull_varattnos(indexPredicate,1,&indexattrs);
47784800

47794801
index_close(indexDesc,AccessShareLock);
47804802
}

‎src/test/subscription/t/100_bugs.pl

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# Tests for various bugs found over time
2+
use strict;
3+
use warnings;
4+
use PostgresNode;
5+
use TestLib;
6+
use Test::Moretests=> 1;
7+
8+
# Bug #15114
9+
10+
# The bug was that determining which columns are part of the replica
11+
# identity index using RelationGetIndexAttrBitmap() would run
12+
# eval_const_expressions() on index expressions and predicates across
13+
# all indexes of the table, which in turn might require a snapshot,
14+
# but there wasn't one set, so it crashes. There were actually two
15+
# separate bugs, one on the publisher and one on the subscriber. The
16+
# fix was to avoid the constant expressions simplification in
17+
# RelationGetIndexAttrBitmap(), so it's safe to call in more contexts.
18+
19+
my$node_publisher = get_new_node('publisher');
20+
$node_publisher->init(allows_streaming=>'logical');
21+
$node_publisher->start;
22+
23+
my$node_subscriber = get_new_node('subscriber');
24+
$node_subscriber->init(allows_streaming=>'logical');
25+
$node_subscriber->start;
26+
27+
my$publisher_connstr =$node_publisher->connstr .' dbname=postgres';
28+
29+
$node_publisher->safe_psql('postgres',
30+
"CREATE TABLE tab1 (a int PRIMARY KEY, b int)");
31+
32+
$node_publisher->safe_psql('postgres',
33+
"CREATE FUNCTION double(x int) RETURNS int IMMUTABLE LANGUAGE SQL AS 'select x * 2'");
34+
35+
# an index with a predicate that lends itself to constant expressions
36+
# evaluation
37+
$node_publisher->safe_psql('postgres',
38+
"CREATE INDEX ON tab1 (b) WHERE a > double(1)");
39+
40+
# and the same setup on the subscriber
41+
$node_subscriber->safe_psql('postgres',
42+
"CREATE TABLE tab1 (a int PRIMARY KEY, b int)");
43+
44+
$node_subscriber->safe_psql('postgres',
45+
"CREATE FUNCTION double(x int) RETURNS int IMMUTABLE LANGUAGE SQL AS 'select x * 2'");
46+
47+
$node_subscriber->safe_psql('postgres',
48+
"CREATE INDEX ON tab1 (b) WHERE a > double(1)");
49+
50+
$node_publisher->safe_psql('postgres',
51+
"CREATE PUBLICATION pub1 FOR ALL TABLES");
52+
53+
$node_subscriber->safe_psql('postgres',
54+
"CREATE SUBSCRIPTION sub1 CONNECTION '$publisher_connstr application_name=sub1' PUBLICATION pub1");
55+
56+
$node_publisher->wait_for_catchup('sub1');
57+
58+
# This would crash, first on the publisher, and then (if the publisher
59+
# is fixed) on the subscriber.
60+
$node_publisher->safe_psql('postgres',
61+
"INSERT INTO tab1 VALUES (1, 2)");
62+
63+
$node_publisher->wait_for_catchup('sub1');
64+
65+
pass('index predicates do not cause crash');

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp