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

> -----Original Message-----
> From: Tom Lane [mailto:tgl(at)sss(dot)pgh(dot)pa(dot)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 '') 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 "")
> 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(at)tpf(dot)co(dot)jp