]> granicus.if.org Git - postgresql/commit
Avoid touching replica identity index in ExtractReplicaIdentity().
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 2 Sep 2019 20:10:37 +0000 (16:10 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 2 Sep 2019 20:10:37 +0000 (16:10 -0400)
commitf63a5ead9d04467e1c1847bd5e3d87c4dca6cd35
treed4286e8a458f58ddea3a9f33834756a44557d655
parentaef36238587c95934185d29ec94e970796f477d8
Avoid touching replica identity index in ExtractReplicaIdentity().

In what seems like a fit of misplaced optimization,
ExtractReplicaIdentity() accessed the relation's replica-identity
index without taking any lock on it.  Usually, the surrounding query
already holds some lock so this is safe enough ... but in the case
of a previously-planned delete, there might be no existing lock.
Given a suitable test case, this is exposed in v12 and HEAD by an
assertion added by commit b04aeb0a0.

The whole thing's rather poorly thought out anyway; rather than
looking directly at the index, we should use the index-attributes
bitmap that's held by the parent table's relcache entry, as the
caller functions do.  This is more consistent and likely a bit
faster, since it avoids a cache lookup.  Hence, change to doing it
that way.

While at it, rather than blithely assuming that the identity
columns are non-null (with catastrophic results if that's wrong),
add assertion checks that they aren't null.  Possibly those should
be 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 was
introduced (in 9.4).  In practice, the risk seems quite low, since
we do have a lock on the index's parent table, so concurrent
changes to the index's catalog entries seem unlikely.  Given the
precedent that commit 9c703c169 wasn't back-patched, I won't risk
back-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
src/backend/access/heap/heapam.c