[HACKERS] [PATCH] Call RelationDropStorage() for broader range of object drops. - Mailing list pgsql-hackers

From Hadi Moshayedi
Subject [HACKERS] [PATCH] Call RelationDropStorage() for broader range of object drops.
Date
Msg-id CA+_kT_cFY-R_z4hwtjTn6fVQas9L6mpqA4V5Uovv8eP4tuMmmg@mail.gmail.com
Whole thread Raw
Responses Re: [HACKERS] [PATCH] Call RelationDropStorage() for broader range ofobject drops.
List 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

pgsql-hackers by date:

Previous
From: Konstantin Knizhnik
Date:
Subject: Re: [HACKERS] Surjective functional indexes
Next
From: Fabien COELHO
Date:
Subject: Re: [HACKERS] pgbench regression test failure