Re: WARNING: relcache reference leak: relation "p1" not closed

On 2017/03/07 10:49, Michael Paquier wrote:
> On Tue, Mar 7, 2017 at 10:37 AM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> On 2017/03/07 7:28, Tom Lane wrote:
>>> Kevin Grittner <kgrittn@gmail.com> writes:
>>>> With e434ad39ae7316bcf35fd578dd34ad7e1ff3c25f I did a `make world`,
>>>> `make install-world`, a fresh default initdb, a start with default
>>>> config, `make installcheck`, connected to the regression database
>>>> with psql as the initial superuser, and ran:
>>>
>>>> regression=# vacuum freeze analyze;
>>>> WARNING:  relcache reference leak: relation "p1" not closed
>>>> VACUUM
>>>
>>> p1 is a partitioned table.  (BTW, could I lobby for people not to use such
>>> generic, collision-prone names for tables that will be left behind after
>>> the regression tests?)  Also, I find that "vacuum analyze" is sufficient,
>>> or even just "analyze", or "analyze p1".  I think it's highly likely this
>>> was introduced by 3c3bb99330aa9b4c2f6258bfa0265d806bf365c3.  Certainly
>>> that failed to add appropriate regression test cases, or we would have
>>> noticed this already.
>>
>> That's right, sorry about that.  Attached patch fixes the relcache leak
>> and adds tests in vacuum.sql and truncate.sql.
> 
> (I was just poking at that)
>              if (childrel != onerel)
>                  heap_close(childrel, AccessShareLock);
> +            else
> +                heap_close(childrel, NoLock);
>              continue;
> Shouldn't that be conditional on the relkind of childrel?

I think we could simply Assert that childrel is partitioned table in this
whole block.  A child table could be a regular table, a materialized view
(?), a foreign table and a partitioned table, the first three of which are
handled by the first two blocks.

Updated patch attached.

Also, I found out that alter_table.sql mistakenly forgot to drop
partitioned table "p1".  Patch 0002 takes care of that.

Thanks,
Amit