RE: [HACKERS] SELECT FOR UPDATE leaks relation refcounts

> -----Original Message-----
> From: owner-pgsql-hackers@postgreSQL.org
> [mailto:owner-pgsql-hackers@postgreSQL.org]On Behalf Of Tom Lane
> 
> Poking into Oliver's report of "RelationClearRelation: relation 21645
> modified while in use", I find that the culprit is the following
> code in execMain.c's InitPlan():
> 
>         foreach(l, parseTree->rowMark)
>         {
>             rm = lfirst(l);
>             relid = rt_fetch(rm->rti, rangeTable)->relid;
>             relation = heap_open(relid, RowShareLock);
>             if (!(rm->info & ROW_MARK_FOR_UPDATE))
>                 continue;
>             erm = (execRowMark *) palloc(sizeof(execRowMark));
>             erm->relation = relation;
>             erm->rti = rm->rti;
>             sprintf(erm->resname, "ctid%u", rm->rti);
>             estate->es_rowMark = lappend(estate->es_rowMark, erm);
>         }
> 
> That heap_open() call has no corresponding heap_close() anywhere,
> so every SELECT FOR UPDATE leaves the relation's refcount one higher
> than it was.  This didn't use to be a huge problem, other than that the
> rel would be permanently locked into the backend's relcache.  (I think
> an attempt to DROP the table later in the session would have caused
> trouble, though.)  However, I just committed changes in the relcache
> that assume that zero refcount is trustworthy, and it's those changes
> that are spitting up.
> 
> It's easy enough to add code to EndPlan that goes through the
> estate->es_rowMark list to close the rels that had ROW_MARK_FOR_UPDATE
> set.  But if that bit wasn't set, the above code opens the rel and then
> forgets about it completely.  Is that a bug?  If not, I guess we need

Seems its a bug though I'm not sure.
Is there anything wrong with inserting heap_close(relation, NoLock)
immediately before 'continue;' ?

Regards.

Hiroshi Inoue
Inoue@tpf.co.jp