Re: [PATCH] Call RelationDropStorage() for broader range of object drops.

On Wed, Sep 13, 2017 at 2:40 AM, Hadi Moshayedi <hadi(at)citusdata(dot)com> wrote:
> Motivation for this patch is that some FDWs (notably, cstore_fdw) try
> utilizing PostgreSQL internal storage. PostgreSQL assigns relfilenode's to
> foreign tables, but doesn't clean up storage for foreign tables when
> dropping tables. Therefore, in cstore_fdw we have to do some tricks to
> handle dropping objects that lead to dropping of cstore table properly.

Foreign tables do not have physical storage assigned to by default. At
least heap_create() tells so, create_storage being set to false for a
foreign table. So there is nothing to clean up normally. Or is
cstore_fdw using directly heap_create with its own relfilenode set,
creating a physical storage?

> So I am suggesting to change the check at heap_drop_with_catalog() at
> src/backend/catalog/heap.c:
>
> - if (rel->rd_rel->relkind != RELKIND_VIEW &&
> - rel->rd_rel->relkind != RELKIND_COMPOSITE_TYPE &&
> - rel->rd_rel->relkind != RELKIND_FOREIGN_TABLE &&
> - rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
> + if (OidIsValid(rel->rd_node.relNode))
> {
> RelationDropStorage(rel);
> }
>
> Any feedback on this?

I agree that there is an inconsistency here if a module calls
heap_create() with an enforced relfilenode.
--
Michael