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

> 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.

This is a good analysis.

> 
> 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.

This is an excellent point.  We know we have some instability in
creating/droping tables in separate sessions at the same time.  This may
not fix that, but it is clearly an issue that an SI message could arrive
at that time.

> 
> 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.

I know I am responsible for at least one of those function calls.  I
remember asking about it in the past.  I have added my first two emails
about this below.  I may have added it whenever I did heap_update
because I never knew what it did, and the name was confusing to me.

---------------------------------------------------------------------------

From maillist Fri Aug 14 22:22:06 1998Date: Fri, 14 Aug 1998 22:22:06 -0400 (EDT)X-Mailer: ELM [version 2.4ME+ PL43
(25)]MIME-Version:1.0Content-Type: text/plain; charset=US-ASCIIContent-Transfer-Encoding: 7bitXFMstatus: 0000To:
(PostgreSQL-development)<hackers@postgreSQL.org>Subject: setheapoverrideContent-Length:   346Status: ROCan someone tell
mewhat setheapoverride() does?  I see it aroundheap_replace a lot.
 

---------------------------------------------------------------------------
Date: Sat, 18 Sep 1999 17:25:43 -0400 (EDT)X-Mailer: ELM [version 2.4ME+ PL56 (25)]MIME-Version: 1.0Content-Type:
text/plain;charset=US-ASCIIContent-Transfer-Encoding: 7bitXFMstatus: 0000To: Tom Lane <tgl@sss.pgh.pa.us>Subject: Re:
[HACKERS]setheapoverride() considered harmfulCc: pgsql-hackers@postgreSQL.orgContent-Length:   628Status: RO> I think
weneed to get rid of setheapoverride().I have always wondered what it did.  It is in my personal TODO with
aquestionmark. Never figured out its purpose.> since this way the tuples still look valid if we look at them again>
laterin the same command.> > Comments?  Anyone know a reason not to get rid of setheapoverride?Yes, please remove it.
 

--  Bruce Momjian                        |  http://www.op.net/~candle pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026