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

> -----Original Message-----
> From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
>
> >>>> Oh,I was just looking at heapoverride stuff quite accidentally.
> >>>> Yes, this call is ugly and should be replaced by
> CommandCounterIncrement()
> >>
> >> OK, I'm running a build now with setheapoverride calls removed.
> >> Will see what happens.
>
> Well, it seems to work, but...
>
> >> About half of the setheapoverride calls surrounded heap_update()
> >> (formerly called heap_replace()) calls.  AFAICS there is no need
> >> for these calls unless heap_update itself needs them --- but there
> >> are many calls to heap_update that do not have setheapoverride.
>
> I figured out that the cases where setheapoverride (or, now,
> CommandCounterIncrement) were needed were the cases where the
> heap_update might be updating a tuple created earlier in the
> same command.  pg_operator.c has some cases like that, but many of
> the other uses of setheapoverride seem to be unnecessary.
>
> However, I'm a little uncomfortable with committing this change,
> because my first try at it worked OK at creating things but fell
> right over on DROP TABLE.  I had replaced the setheapoverride(true)
> in heap_drop_with_catalog() (backend/catalog/heap.c) with a
> CommandCounterIncrement, and it failed with this backtrace:
>
> (gdb) bt
> #0  elog (lev=-1, fmt=0x6b160 "cannot find attribute %d of relation %s")
>     at elog.c:112
> #1  0x1767d8 in build_tupdesc_ind (buildinfo={infotype = 1, i = {
>         info_id = 19040,
>         info_name = 0x4a60}},
>     relation=0x4006ef00, natts=1074198864) at relcache.c:527
> #2  0x176554 in RelationBuildTupleDesc (buildinfo={infotype = 1, i = {
>         info_id = 19040,
>         info_name = 0x4a60 }},
>     relation=0x1, natts=1074198864) at relcache.c:437
> #3  0x177230 in RelationBuildDesc (buildinfo={infotype = 1, i = {
>         info_id = 19040,
>         info_name = 0x4a60 }},
>     oldrelation=0x4006ef00) at relcache.c:808
> #4  0x177b28 in RelationClearRelation (relation=0x4006ef00,
> rebuildIt=0 '\000')
>     at relcache.c:1279
> #5  0x177bbc in RelationFlushRelation (relationPtr=0xffffffff,
>     onlyFlushReferenceCountZero=96 '`') at relcache.c:1320
> #6  0x177e10 in RelationIdInvalidateRelationCacheByRelationId (
>     relationId=19040) at relcache.c:1415
> #7  0x175968 in CacheIdInvalidate (cacheId=4294967295, hashIndex=438624,
>     pointer=0x1) at inval.c:544
> #8  0x175ae8 in InvalidationMessageCacheInvalidate (message=0x4007cce4)
>     at inval.c:657
> #9  0x175490 in LocalInvalidInvalidate (invalid=0x4007cce4 "r",
>     function=0x4000c3ca <DINFINITY+9226>, freemember=1 '\001') at
> inval.c:173
> #10 0x175ca4 in ImmediateLocalInvalidation (send=-1 '\xA0') at inval.c:806
> #11 0x9d0b0 in AtCommit_LocalCache () at xact.c:687
> #12 0x9cf70 in CommandCounterIncrement () at xact.c:520
> #13 0xa7a08 in heap_drop_with_catalog (relname=0x4006ef00 "\xA0\xA0\xA0\xA0")
>     at heap.c:1528
> #14 0xb16ac in RemoveRelation (name=0x40083328 "ff1") at creatinh.c:217
> #15 0x13ce84 in ProcessUtility (parsetree=0x40083370, dest=Remote)
>     at utility.c:201
> #16 0x13add0 in pg_exec_query_dest (query_string=0x40024270 "drop
> table ff1",
>     dest=Remote, aclOverride=1 '\001') at postgres.c:721
>
> 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().
Does anyone know ?
If no one knows,we had better remove setheapoverride().

Regards.

Hiroshi Inoue
Inoue@tpf.co.jp