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

From: Hadi Moshayedi <hadi(at)citusdata(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: [PATCH] Call RelationDropStorage() for broader range of object drops.
Date: 2017-09-12 17:40:43
Message-ID: CA+_kT_cFY-R_z4hwtjTn6fVQas9L6mpqA4V5Uovv8eP4tuMmmg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

As far as I can see in the code, the requirement for
RelationDropStorage(rel) is having valid rel->rd_node.relNode, but it
doesn't actually require the storage files to actually exist. We don't emit
warning messages in mdunlinkfork() if the result of unlink() is ENOENT. So
we can RelationDropStorage() regardless of storage files existing or not,
given that the relation has valid relfilenode.

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?

Thanks,
Hadi

Attachment Content-Type Size
drop_storage.patch text/x-patch 589 bytes

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2017-09-12 18:00:56 Re: pgbench regression test failure
Previous Message Konstantin Knizhnik 2017-09-12 17:39:09 Re: Surjective functional indexes