Re: [HACKERS] Commits 8de72b and 5457a1 (COPY FREEZE)

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2019-05-01 14:44:12 -0400, Tom Lane wrote:
>> There might be an argument for treating rd_createSubid the same way,
>> not sure. That field wouldn't ever be set on a system catalog, so
>> it's less of a hazard, but there's something to be said for treating
>> the two fields alike.

> Seems sensible to treat it the same way. Not sure if there's an actual
> problem where the current treatment could cause a problem, but seems
> worth improving.

So if I try to treat rd_createSubid the same way, it passes regression
in normal mode, but CLOBBER_CACHE_ALWAYS is a total disaster: all
table creations fail, eg

regression=# CREATE TABLE BOOLTBL1 (f1 bool);
psql: ERROR: relation 92773 deleted while still in use

What is happening is that once heap_create has created a relcache
entry for the new relation, any RelationCacheInvalidate call causes
us to try to reload that relcache entry, and that fails because there's
no pg_class or pg_attribute entries for the new relation yet.

This gets back to the thing we were poking at in the other thread,
which is whether or not a relcache entry is *always* reconstructible
from on-disk state. I had in the back of my mind "um, what about
initial table creation?" but hadn't looked closely. It seems that
the only reason that it works is that RelationCacheInvalidate is
unwilling to touch new-in-transaction relcache entries.

The ideal fix for this, perhaps, would be to rejigger things far enough
that we would not try to create a relcache entry for a new rel until
it was supported by some minimal set of catalog entries. But that's
more change than I really want to undertake right now, and for sure
I wouldn't want to back-patch it.

Anyway, I believe the original justification for skipping
new-in-transaction entries here, which is that they couldn't possibly be
subjects of any cross-backend notification messages. I think the argument
that that applies to rd_newRelfilenodeSubid is a whole lot weaker though.
But I'm going to let this go for the moment, because it's not clear whether
a patch like this is actually relevant to our immediate problem. There
seem to be some other order-of-operations issues in invalidation
processing, and maybe fixing those would suffice. More later.

regards, tom lane