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

Commitcf25498

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 parentff9e63c commitcf25498

File tree

2 files changed

+106
-19
lines changed

2 files changed

+106
-19
lines changed

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

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
#include"access/xact.h"
4141
#include"access/xlog.h"
4242
#include"catalog/catalog.h"
43-
#include"catalog/index.h"
4443
#include"catalog/indexing.h"
4544
#include"catalog/namespace.h"
4645
#include"catalog/partition.h"
@@ -4700,12 +4699,12 @@ RelationGetIndexPredicate(Relation relation)
47004699
* 2. "recheck_on_update" index option explicitly set by user, which overrides 1)
47014700
*/
47024701
staticbool
4703-
IsProjectionFunctionalIndex(Relationindex,IndexInfo*ii)
4702+
IsProjectionFunctionalIndex(Relationindex)
47044703
{
47054704
boolis_projection= false;
47064705

47074706
#ifdefNOT_USED
4708-
if (ii->ii_Expressions)
4707+
if (RelationGetIndexExpressions(index))
47094708
{
47104709
HeapTupletuple;
47114710
Datumreloptions;
@@ -4715,7 +4714,7 @@ IsProjectionFunctionalIndex(Relation index, IndexInfo *ii)
47154714
/* by default functional index is considered as non-injective */
47164715
is_projection= true;
47174716

4718-
cost_qual_eval(&index_expr_cost,ii->ii_Expressions,NULL);
4717+
cost_qual_eval(&index_expr_cost,RelationGetIndexExpressions(index),NULL);
47194718

47204719
/*
47214720
* If index expression is too expensive, then disable projection
@@ -4861,21 +4860,44 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
48614860
{
48624861
OidindexOid=lfirst_oid(l);
48634862
RelationindexDesc;
4864-
IndexInfo*indexInfo;
4863+
Datumdatum;
4864+
boolisnull;
4865+
Node*indexExpressions;
4866+
Node*indexPredicate;
48654867
inti;
48664868
boolisKey;/* candidate key */
48674869
boolisPK;/* primary key */
48684870
boolisIDKey;/* replica identity index */
48694871

48704872
indexDesc=index_open(indexOid,AccessShareLock);
48714873

4872-
/* Extract index key information from the index's pg_index row */
4873-
indexInfo=BuildIndexInfo(indexDesc);
4874+
/*
4875+
* Extract index expressions and index predicate. Note: Don't use
4876+
* RelationGetIndexExpressions()/RelationGetIndexPredicate(), because
4877+
* those might run constant expressions evaluation, which needs a
4878+
* snapshot, which we might not have here. (Also, it's probably more
4879+
* sound to collect the bitmaps before any transformations that might
4880+
* eliminate columns, but the practical impact of this is limited.)
4881+
*/
4882+
4883+
datum=heap_getattr(indexDesc->rd_indextuple,Anum_pg_index_indexprs,
4884+
GetPgIndexDescriptor(),&isnull);
4885+
if (!isnull)
4886+
indexExpressions=stringToNode(TextDatumGetCString(datum));
4887+
else
4888+
indexExpressions=NULL;
4889+
4890+
datum=heap_getattr(indexDesc->rd_indextuple,Anum_pg_index_indpred,
4891+
GetPgIndexDescriptor(),&isnull);
4892+
if (!isnull)
4893+
indexPredicate=stringToNode(TextDatumGetCString(datum));
4894+
else
4895+
indexPredicate=NULL;
48744896

48754897
/* Can this index be referenced by a foreign key? */
4876-
isKey=indexInfo->ii_Unique&&
4877-
indexInfo->ii_Expressions==NIL&&
4878-
indexInfo->ii_Predicate==NIL;
4898+
isKey=indexDesc->rd_index->indisunique&&
4899+
indexExpressions==NULL&&
4900+
indexPredicate==NULL;
48794901

48804902
/* Is this a primary key? */
48814903
isPK= (indexOid==relpkindex);
@@ -4884,9 +4906,9 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
48844906
isIDKey= (indexOid==relreplindex);
48854907

48864908
/* Collect simple attribute references */
4887-
for (i=0;i<indexInfo->ii_NumIndexAttrs;i++)
4909+
for (i=0;i<indexDesc->rd_index->indnatts;i++)
48884910
{
4889-
intattrnum=indexInfo->ii_IndexAttrNumbers[i];
4911+
intattrnum=indexDesc->rd_index->indkey.values[i];
48904912

48914913
/*
48924914
* Since we have covering indexes with non-key columns, we must
@@ -4901,33 +4923,33 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
49014923
indexattrs=bms_add_member(indexattrs,
49024924
attrnum-FirstLowInvalidHeapAttributeNumber);
49034925

4904-
if (isKey&&i<indexInfo->ii_NumIndexKeyAttrs)
4926+
if (isKey&&i<indexDesc->rd_index->indnkeyatts)
49054927
uindexattrs=bms_add_member(uindexattrs,
49064928
attrnum-FirstLowInvalidHeapAttributeNumber);
49074929

4908-
if (isPK&&i<indexInfo->ii_NumIndexKeyAttrs)
4930+
if (isPK&&i<indexDesc->rd_index->indnkeyatts)
49094931
pkindexattrs=bms_add_member(pkindexattrs,
49104932
attrnum-FirstLowInvalidHeapAttributeNumber);
49114933

4912-
if (isIDKey&&i<indexInfo->ii_NumIndexKeyAttrs)
4934+
if (isIDKey&&i<indexDesc->rd_index->indnkeyatts)
49134935
idindexattrs=bms_add_member(idindexattrs,
49144936
attrnum-FirstLowInvalidHeapAttributeNumber);
49154937
}
49164938
}
49174939

49184940
/* Collect attributes used in expressions, too */
4919-
if (IsProjectionFunctionalIndex(indexDesc,indexInfo))
4941+
if (IsProjectionFunctionalIndex(indexDesc))
49204942
{
49214943
projindexes=bms_add_member(projindexes,indexno);
4922-
pull_varattnos((Node*)indexInfo->ii_Expressions,1,&projindexattrs);
4944+
pull_varattnos(indexExpressions,1,&projindexattrs);
49234945
}
49244946
else
49254947
{
49264948
/* Collect all attributes used in expressions, too */
4927-
pull_varattnos((Node*)indexInfo->ii_Expressions,1,&indexattrs);
4949+
pull_varattnos(indexExpressions,1,&indexattrs);
49284950
}
49294951
/* Collect all attributes in the index predicate, too */
4930-
pull_varattnos((Node*)indexInfo->ii_Predicate,1,&indexattrs);
4952+
pull_varattnos(indexPredicate,1,&indexattrs);
49314953

49324954
index_close(indexDesc,AccessShareLock);
49334955
indexno+=1;

‎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