On Wed, Aug 26, 2020 at 09:08:51PM +0900, Michael Paquier wrote:
> I'll go through this part again tomorrow, it is late here.
There is actually a huge gotcha here that exists since replica
identities exist: relation_mark_replica_identity() only considers
valid indexes! So, on HEAD, if DROP INDEX CONCURRENTLY fails in the
second transaction doing the physical drop, then we would finish with
a catalog entry that has indisreplident and !indisinvalid. If you
reindex it afterwards and switch the index back to be valid, there
can be more than one valid index marked with indisreplident, which
messes up with the logic in tablecmds.c because we use
RelationGetIndexList(), that discards invalid indexes. This case is
actually rather similar to the handling of indisclustered.
So we have a set of two issues here:
1) indisreplident should be switched to false when we clear the valid
flag of an index for INDEX_DROP_CLEAR_VALID. This issue exists since
9.4.
2) To never finish in a state where we have REPLICA IDENTITY INDEX
without an index marked as indisreplident, we need to reset the
replident of the parent table in the same transaction as the one
clearing indisvalid for the concurrent case. That's a problem of the
patch proposed upthread.
Attached is a patch for 1) and 2) grouped together, to ease review for
now. I think that we had better fix 1) separately though, so I am
going to start a new thread about that with a separate patch as the
current thread is misleading.
--
Michael