Re: [HACKERS] RE: Getting rid of setheapoverride (was Re: [COMMITTERS] heap.c)

"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:
>> Apparently, if I call CommandCounterIncrement while partway through
>> dropping a relation, it will try to rebuild the relcache entry for
>> the relation --- and of course fail.  I'm too tired to figure out
>> whether this is just a small coding error in the new cache invalidation
>> code or whether it's a serious limitation in the whole design.  Hiroshi,
>> what do you think?

> Hmmm,CommandCounterIncrement() was about to rebuild relation decriptor
> for the half dropped table and failed. It seems impossible to call Command-
> CounterIncrement() in heap_drop_with_catalog.

> I don't understand why DeleteTypeTuple() require setheapoverride().

As far as I can tell, it doesn't --- drop table seems to work just fine
without it.

I have been thinking some more about this, and have come to the
conclusion that it is only safe to call CommandCounterIncrement
at points where you have a self-consistent catalog tuple state.
In particular it must be possible to build valid relcache entries
from whatever tuples you are making visible with the increment.

For example, it's OK for heap_create to call C.C.I. after creating the
relation's pg_class, pg_type, and pg_attribute entries; the relation
is not finished, since it lacks default and constraint info, but
relcache.c won't have a problem with that.  (Note that heap_create
explicitly forces a rebuild of the relcache entry after it's added
that extra stuff!)

It is *not* OK for heap_drop to call C.C.I. where it was doing it,
because it had already deleted the pg_attribute tuples, but was still
holding a refcount lock on the relcache entry for the target relation.
(If the refcount were zero, then relcache.c would have just dropped
the entry instead of trying to rebuild it...)

The heap_drop code was risky even in its original form of
setheapoverride, since had a relcache rebuild been caused during
DeleteTypeTuple(), it would have failed.  (This could happen,
in the current state of the code, if an SI Reset message arrives
and gets processed while DeleteTypeTuple is trying to open pg_type.)
Switching to CommandCounterIncrement just exposed the latent bug
by forcing the rebuild attempt to occur.

In short, I have convinced myself that this is all fine.  I will
finish ripping out setheapoverride and commit the changes tonight.
Should be able to simplify tqual.c a little bit now that we don't
need the override code.
        regards, tom lane