On Wed, Jul 12, 2023 at 04:02:23PM -0400, Robert Haas wrote: > Oh, interesting. The fact that indisreplident isn't copied seems like > a pretty clear mistake, but I'm guessing that the fact that t_self > wasn't refreshed was deliberate and that the author of this code > didn't really intend for callers to look at the t_self value. We could > change our mind about whether that ought to be allowed, though. But, > like, none of the other tuple header fields are copied either... xmax, > xvac, infomask, etc.
See 3c84046 and 8ec9438, mainly, from Tom. I didn't know that this is used as a shortcut to reload index information in the cache because it is much cheaper than a full index information rebuild. I agree that not copying indisreplident in this code path is a mistake as this bug shows, because any follow-up command run in a transaction that changed this field would get an incorrect information reference. Now, I have to admit that I am not completely sure what the consequences of this addition are when it comes to concurrent index operations (CREATE/DROP INDEX, REINDEX): /* Copy xmin too, as that is needed to make sense of indcheckxmin */ HeapTupleHeaderSetXmin(relation->rd_indextuple->t_data, HeapTupleHeaderGetXmin(tuple->t_data)); + ItemPointerCopy(&tuple->t_self, &relation->rd_indextuple->t_self); Anyway, as I have pointed upthread, I think that the craziness is also in validatePartitionedIndex() where this stuff thinks that it is OK to use a copy the pg_index tuple coming from the relcache. As this report proves, it is *not* safe, because we may miss a lot of information not updated by RelationReloadIndexInfo() that other commands in the same transaction block may have updated, and the code would insert into the catalog an inconsistent tuple for a partitioned index switched to become valid. I agree that updating indisreplident in this cheap index reload path is necessary, as well. Does my suggestion of using the syscache not make sense for somebody here? Note that this is what all the other code paths do for catalog updates of pg_index when retrieving a copy of its tuples. -- Michael
signature.asc
Description: PGP signature