From 5023c5d9deea7008e4b74f02564262714fb8c6b6 Mon Sep 17 00:00:00 2001 From: Mats Kindahl Date: Mon, 31 Oct 2022 14:27:03 +0100 Subject: [PATCH] Add callback for unlinking file node of table When dropping or truncating a table, the old file locator is unlinked and a new file locator created for the relation. This is, however, done without notifying the table access method that the unlinking is taking place. This becomes a problem if the table access method has an internal resource associated with the file locator that also needs to be unlinked or cleaned up. This commit add a callback to the table access method for resetting the file locator when it is dropped and call it to unlink the file locator so that the table access method and unlink the resource internally, if necessary. The callback can be undefined (that is, NULL) if the table access method do not require notification that a resource is unlinked. --- src/backend/access/heap/heapam_handler.c | 7 +++++++ src/backend/catalog/heap.c | 22 ++++++++++++++++++++- src/backend/utils/cache/relcache.c | 7 ++++++- src/include/access/tableam.h | 25 ++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 41f1ca65d0..d67b1dcfbf 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -615,6 +615,12 @@ heapam_relation_set_new_filelocator(Relation rel, smgrclose(srel); } +static void +heapam_relation_reset_filelocator(Relation rel) +{ + RelationDropStorage(rel); +} + static void heapam_relation_nontransactional_truncate(Relation rel) { @@ -2566,6 +2572,7 @@ static const TableAmRoutine heapam_methods = { .index_delete_tuples = heap_index_delete_tuples, .relation_set_new_filelocator = heapam_relation_set_new_filelocator, + .relation_reset_filelocator = heapam_relation_reset_filelocator, .relation_nontransactional_truncate = heapam_relation_nontransactional_truncate, .relation_copy_data = heapam_relation_copy_data, .relation_copy_for_cluster = heapam_relation_copy_for_cluster, diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 5b49cc5a09..1d1379424f 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -1850,7 +1850,27 @@ heap_drop_with_catalog(Oid relid) * Schedule unlinking of the relation's physical files at commit. */ if (RELKIND_HAS_STORAGE(rel->rd_rel->relkind)) - RelationDropStorage(rel); + switch (rel->rd_rel->relkind) + { + case RELKIND_VIEW: + case RELKIND_COMPOSITE_TYPE: + case RELKIND_FOREIGN_TABLE: + case RELKIND_PARTITIONED_TABLE: + case RELKIND_PARTITIONED_INDEX: + Assert(false); + break; + + case RELKIND_INDEX: + case RELKIND_SEQUENCE: + RelationDropStorage(rel); + break; + + case RELKIND_RELATION: + case RELKIND_TOASTVALUE: + case RELKIND_MATVIEW: + table_relation_reset_filelocator(rel); + break; + } /* ensure that stats are dropped if transaction commits */ pgstat_drop_relation(rel); diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index bd6cd4e47b..d39c57b855 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -3778,9 +3778,14 @@ RelationSetNewRelfilenumber(Relation relation, char persistence) smgrdounlinkall(&srel, 1, false); smgrclose(srel); } + if (RELKIND_HAS_TABLE_AM(relation->rd_rel->relkind)) + { + table_relation_reset_filelocator(relation); + } else { - /* Not a binary upgrade, so just schedule it to happen later. */ + /* Not a binary upgrade and not a relation with a table access method, + * so just schedule it to happen later. */ RelationDropStorage(relation); } diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index e45d73eae3..5141f9b3b7 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -581,6 +581,15 @@ typedef struct TableAmRoutine TransactionId *freezeXid, MultiXactId *minmulti); + /* + * This callback needs to schedule the removal of all associations with + * the relation `rel` since the relation is about to be dropped or the + * storage recycled for other reasons. + * + * See also table_relation_reset_filelocator(). + */ + void (*relation_reset_filelocator) (Relation rel); + /* * This callback needs to remove all contents from `rel`'s current * relfilelocator. No provisions for transactional behaviour need to be @@ -1600,6 +1609,22 @@ table_relation_set_new_filelocator(Relation rel, minmulti); } +/* + * Schedule the removal of all association with storage for the relation. + * + * This is used when a relation is about to be dropped and removed from the + * catalog. + * + * Since not all access methods require this callback, we do not require the + * reset callback to be defined and only call it if it is actually available. + */ +static inline void +table_relation_reset_filelocator(Relation rel) +{ + if (rel->rd_tableam) + rel->rd_tableam->relation_reset_filelocator(rel); +} + /* * Remove all table contents from `rel`, in a non-transactional manner. * Non-transactional meaning that there's no need to support rollbacks. This -- 2.34.1