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

Commitaad0926

Browse files
committed
Avoid touching replica identity index in ExtractReplicaIdentity().
In what seems like a fit of misplaced optimization,ExtractReplicaIdentity() accessed the relation's replica-identityindex without taking any lock on it. Usually, the surrounding queryalready holds some lock so this is safe enough ... but in the caseof a previously-planned delete, there might be no existing lock.Given a suitable test case, this is exposed in v12 and HEAD by anassertion added by commitb04aeb0.The whole thing's rather poorly thought out anyway; rather thanlooking directly at the index, we should use the index-attributesbitmap that's held by the parent table's relcache entry, as thecaller functions do. This is more consistent and likely a bitfaster, since it avoids a cache lookup. Hence, change to doing itthat way.While at it, rather than blithely assuming that the identitycolumns are non-null (with catastrophic results if that's wrong),add assertion checks that they aren't null. Possibly those shouldbe actual test-and-elog, but I'll leave it like this for now.In principle, this is a bug that's been there since this code wasintroduced (in 9.4). In practice, the risk seems quite low, sincewe do have a lock on the index's parent table, so concurrentchanges to the index's catalog entries seem unlikely. Given theprecedent that commit9c703c1 wasn't back-patched, I won't riskback-patching this further than v12.Per report from Hadi Moshayedi.Discussion:https://postgr.es/m/CAK=1=Wrek44Ese1V7LjKiQS-Nd-5LgLi_5_CskGbpggKEf3tKQ@mail.gmail.com
1 parent90433c3 commitaad0926

File tree

1 file changed

+37
-34
lines changed

1 file changed

+37
-34
lines changed

‎src/backend/access/heap/heapam.c

Lines changed: 37 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -7593,19 +7593,24 @@ log_heap_new_cid(Relation relation, HeapTuple tup)
75937593
* the old tuple in a UPDATE or DELETE.
75947594
*
75957595
* Returns NULL if there's no need to log an identity or if there's no suitable
7596-
* key in the Relation relation.
7596+
* key defined.
7597+
*
7598+
* key_changed should be false if caller knows that no replica identity
7599+
* columns changed value. It's always true in the DELETE case.
7600+
*
7601+
* *copy is set to true if the returned tuple is a modified copy rather than
7602+
* the same tuple that was passed in.
75977603
*/
75987604
staticHeapTuple
7599-
ExtractReplicaIdentity(Relationrelation,HeapTupletp,boolkey_changed,bool*copy)
7605+
ExtractReplicaIdentity(Relationrelation,HeapTupletp,boolkey_changed,
7606+
bool*copy)
76007607
{
76017608
TupleDescdesc=RelationGetDescr(relation);
7602-
Oidreplidindex;
7603-
Relationidx_rel;
76047609
charreplident=relation->rd_rel->relreplident;
7605-
HeapTuplekey_tuple=NULL;
7610+
Bitmapset*idattrs;
7611+
HeapTuplekey_tuple;
76067612
boolnulls[MaxHeapAttributeNumber];
76077613
Datumvalues[MaxHeapAttributeNumber];
7608-
intnatt;
76097614

76107615
*copy= false;
76117616

@@ -7624,7 +7629,7 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_changed, bool *
76247629
if (HeapTupleHasExternal(tp))
76257630
{
76267631
*copy= true;
7627-
tp=toast_flatten_tuple(tp,RelationGetDescr(relation));
7632+
tp=toast_flatten_tuple(tp,desc);
76287633
}
76297634
returntp;
76307635
}
@@ -7633,41 +7638,39 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_changed, bool *
76337638
if (!key_changed)
76347639
returnNULL;
76357640

7636-
/* find the replica identity index */
7637-
replidindex=RelationGetReplicaIndex(relation);
7638-
if (!OidIsValid(replidindex))
7639-
{
7640-
elog(DEBUG4,"could not find configured replica identity for table \"%s\"",
7641-
RelationGetRelationName(relation));
7642-
returnNULL;
7643-
}
7644-
7645-
idx_rel=RelationIdGetRelation(replidindex);
7646-
7647-
Assert(CheckRelationLockedByMe(idx_rel,AccessShareLock, true));
7648-
7649-
/* deform tuple, so we have fast access to columns */
7650-
heap_deform_tuple(tp,desc,values,nulls);
7641+
/* find out the replica identity columns */
7642+
idattrs=RelationGetIndexAttrBitmap(relation,
7643+
INDEX_ATTR_BITMAP_IDENTITY_KEY);
76517644

7652-
/* set all columns to NULL, regardless of whether they actually are */
7653-
memset(nulls,1,sizeof(nulls));
7645+
/*
7646+
* If there's no defined replica identity columns, treat as !key_changed.
7647+
* (This case should not be reachable from heap_update, since that should
7648+
* calculate key_changed accurately. But heap_delete just passes constant
7649+
* true for key_changed, so we can hit this case in deletes.)
7650+
*/
7651+
if (bms_is_empty(idattrs))
7652+
returnNULL;
76547653

76557654
/*
7656-
* Now set all columns contained in the index to NOT NULL, they cannot
7657-
* currently be NULL.
7655+
* Construct a new tuple containing only the replica identity columns,
7656+
* with nulls elsewhere. While we're at it, assert that the replica
7657+
* identity columns aren't null.
76587658
*/
7659-
for (natt=0;natt<IndexRelationGetNumberOfKeyAttributes(idx_rel);natt++)
7660-
{
7661-
intattno=idx_rel->rd_index->indkey.values[natt];
7659+
heap_deform_tuple(tp,desc,values,nulls);
76627660

7663-
if (attno<0)
7664-
elog(ERROR,"system column in index");
7665-
nulls[attno-1]= false;
7661+
for (inti=0;i<desc->natts;i++)
7662+
{
7663+
if (bms_is_member(i+1-FirstLowInvalidHeapAttributeNumber,
7664+
idattrs))
7665+
Assert(!nulls[i]);
7666+
else
7667+
nulls[i]= true;
76667668
}
76677669

76687670
key_tuple=heap_form_tuple(desc,values,nulls);
76697671
*copy= true;
7670-
RelationClose(idx_rel);
7672+
7673+
bms_free(idattrs);
76717674

76727675
/*
76737676
* If the tuple, which by here only contains indexed columns, still has
@@ -7680,7 +7683,7 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_changed, bool *
76807683
{
76817684
HeapTupleoldtup=key_tuple;
76827685

7683-
key_tuple=toast_flatten_tuple(oldtup,RelationGetDescr(relation));
7686+
key_tuple=toast_flatten_tuple(oldtup,desc);
76847687
heap_freetuple(oldtup);
76857688
}
76867689

0 commit comments

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