RE: refcnt leak ?

> -----Original Message-----
> From: Tom Lane [mailto:tgl(at)sss(dot)pgh(dot)pa(dot)us]
>
> Hiroshi Inoue <Inoue(at)tpf(dot)co(dot)jp> writes:
> > While examining recursive use of catalog cache,I found
> > a refcnt leak of relations.
> > After further investigation,I found that the following seems
> > to be the cause.
>
> > [ in EndAppend() in nodeAppend.c ]
>
> appendstate-> as_result_relation_info_list = NIL;
>

Oh no,my point isn't on this line but on the line

estate->es_result_relation_info = NULL;

As the comment says,it depends on the assumption that
estate->es_result_relation_info points to one of the node
of appendstate->as_result_relation_info_list(before set to
NIL). However ISTM appendstate->as_result_relation_info
_list is for inheritance and in the case "INSERT INTO ..
SELECT .. FROM .." it's not used.

> That doesn't look like a problem to me --- the result relations *have*
> been closed, just above this line.
>
> > BTW,doesn't EndAppend() neglect to call ExecCloseIndices()
> > for RelationInfos of appendstate->as_result_relation_info_list ?
>
> Comparing nodeAppend to EndPlan(), I think you are right --- each
> resultinfo should have ExecCloseIndices applied too, in the loop just
> above the line you quote. This did not use to be a problem because
> Append plans were readonly, but now that we have UPDATE/DELETE on
> inheritance hierarchies, there's a missing step here. Was your test
> query of that kind?
>

I first changed this part but rd_refcnt leak didn't disappaear.
I have no refcnt leak example which is caused due to this flaw(?).
After that I found " estate->es_result_relation_info = NULL; "
in EndAppend() . I changed it to not do so when appendstate->
as_result_relation_info_list is NIL and rd_refcnt leak disappeared.

Regards.
Hiroshi Inoue