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

Commit2bac1d8

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 parent11b6345 commit2bac1d8

File tree

2 files changed

+107
-11
lines changed

2 files changed

+107
-11
lines changed

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

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
#include"access/xact.h"
3939
#include"access/xlog.h"
4040
#include"catalog/catalog.h"
41-
#include"catalog/index.h"
4241
#include"catalog/indexing.h"
4342
#include"catalog/namespace.h"
4443
#include"catalog/partition.h"
@@ -4948,21 +4947,44 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
49484947
{
49494948
OidindexOid=lfirst_oid(l);
49504949
RelationindexDesc;
4951-
IndexInfo*indexInfo;
4950+
Datumdatum;
4951+
boolisnull;
4952+
Node*indexExpressions;
4953+
Node*indexPredicate;
49524954
inti;
49534955
boolisKey;/* candidate key */
49544956
boolisPK;/* primary key */
49554957
boolisIDKey;/* replica identity index */
49564958

49574959
indexDesc=index_open(indexOid,AccessShareLock);
49584960

4959-
/* Extract index key information from the index's pg_index row */
4960-
indexInfo=BuildIndexInfo(indexDesc);
4961+
/*
4962+
* Extract index expressions and index predicate. Note: Don't use
4963+
* RelationGetIndexExpressions()/RelationGetIndexPredicate(), because
4964+
* those might run constant expressions evaluation, which needs a
4965+
* snapshot, which we might not have here. (Also, it's probably more
4966+
* sound to collect the bitmaps before any transformations that might
4967+
* eliminate columns, but the practical impact of this is limited.)
4968+
*/
4969+
4970+
datum=heap_getattr(indexDesc->rd_indextuple,Anum_pg_index_indexprs,
4971+
GetPgIndexDescriptor(),&isnull);
4972+
if (!isnull)
4973+
indexExpressions=stringToNode(TextDatumGetCString(datum));
4974+
else
4975+
indexExpressions=NULL;
4976+
4977+
datum=heap_getattr(indexDesc->rd_indextuple,Anum_pg_index_indpred,
4978+
GetPgIndexDescriptor(),&isnull);
4979+
if (!isnull)
4980+
indexPredicate=stringToNode(TextDatumGetCString(datum));
4981+
else
4982+
indexPredicate=NULL;
49614983

49624984
/* Can this index be referenced by a foreign key? */
4963-
isKey=indexInfo->ii_Unique&&
4964-
indexInfo->ii_Expressions==NIL&&
4965-
indexInfo->ii_Predicate==NIL;
4985+
isKey=indexDesc->rd_index->indisunique&&
4986+
indexExpressions==NULL&&
4987+
indexPredicate==NULL;
49664988

49674989
/* Is this a primary key? */
49684990
isPK= (indexOid==relpkindex);
@@ -4971,9 +4993,9 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
49714993
isIDKey= (indexOid==relreplindex);
49724994

49734995
/* Collect simple attribute references */
4974-
for (i=0;i<indexInfo->ii_NumIndexAttrs;i++)
4996+
for (i=0;i<indexDesc->rd_index->indnatts;i++)
49754997
{
4976-
intattrnum=indexInfo->ii_KeyAttrNumbers[i];
4998+
intattrnum=indexDesc->rd_index->indkey.values[i];
49774999

49785000
if (attrnum!=0)
49795001
{
@@ -4995,10 +5017,10 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
49955017
}
49965018

49975019
/* Collect all attributes used in expressions, too */
4998-
pull_varattnos((Node*)indexInfo->ii_Expressions,1,&indexattrs);
5020+
pull_varattnos(indexExpressions,1,&indexattrs);
49995021

50005022
/* Collect all attributes in the index predicate, too */
5001-
pull_varattnos((Node*)indexInfo->ii_Predicate,1,&indexattrs);
5023+
pull_varattnos(indexPredicate,1,&indexattrs);
50025024

50035025
index_close(indexDesc,AccessShareLock);
50045026
}

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

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

0 commit comments

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