From 938a8456ec6721a5f1f703741b421e90f2b6fd38 Mon Sep 17 00:00:00 2001 From: Vignesh C Date: Tue, 13 Oct 2026 10:15:06 +0530 Subject: [PATCH v2] Avoid cache lookup failure for concurrently dropped relations InvalidatePublicationRels() is given relation OIDs that several callers collect without holding a lock on the relations: DROP PUBLICATION (both the FOR TABLE and the FOR TABLES IN SCHEMA cases), ALTER PUBLICATION ... SET (...), and ALTER PUBLICATION ... ADD TABLES IN SCHEMA. A partition or schema table can be dropped after the list was built, leaving its OID in the list. Invalidating the relcache for such a relation then fails with "cache lookup failed for relation". Add a missing_ok argument to InvalidatePublicationRels() and skip relations that no longer exist when it is true. All callers pass true except publication_add_relation(), whose caller holds ShareUpdateExclusiveLock on the published table until the end of the command; that also keeps its partitions from being dropped, so a missing relation there would indicate a bug and is still reported. --- src/backend/catalog/pg_publication.c | 8 ++++-- src/backend/commands/publicationcmds.c | 34 ++++++++++++++++++++------ src/include/commands/publicationcmds.h | 2 +- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index f11ea8839ca..1d09d4fc37c 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -684,7 +684,11 @@ publication_add_relation(Oid pubid, PublicationRelInfo *pri, relids = GetPubPartitionOptionRelations(relids, PUBLICATION_PART_ALL, relid); - InvalidatePublicationRels(relids); + /* + * The relation is locked by the caller, which also keeps its + * partitions from being dropped. + */ + InvalidatePublicationRels(relids, false); } return myself; @@ -889,7 +893,7 @@ publication_add_schema(Oid pubid, Oid schemaid, bool if_not_exists) */ schemaRels = GetSchemaPublicationRelations(schemaid, PUBLICATION_PART_ALL); - InvalidatePublicationRels(schemaRels); + InvalidatePublicationRels(schemaRels, true); return myself; } diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c index 301050104dd..40620d7c229 100644 --- a/src/backend/commands/publicationcmds.c +++ b/src/backend/commands/publicationcmds.c @@ -1195,7 +1195,7 @@ AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt, PUBLICATION_PART_ALL); relids = list_concat_unique_oid(relids, schemarelids); - InvalidatePublicationRels(relids); + InvalidatePublicationRels(relids, true); } ObjectAddressSet(obj, PublicationRelationId, pubform->oid); @@ -1207,9 +1207,18 @@ AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt, /* * Invalidate the relations. + * + * If missing_ok is true, a relation that no longer exists is ignored. + * + * Callers that build the list without holding a lock that prevents the + * relations from being dropped (DROP PUBLICATION, ALTER PUBLICATION ... + * SET (...), a schema's tables) must pass missing_ok = true, since a relation + * can be dropped concurrently after the list was built. Callers that hold a + * lock on every relation in the list pass false, so that a missing relation + * is still reported. */ void -InvalidatePublicationRels(List *relids) +InvalidatePublicationRels(List *relids, bool missing_ok) { /* * We don't want to send too many individual messages, at some point it's @@ -1217,10 +1226,21 @@ InvalidatePublicationRels(List *relids) */ if (list_length(relids) < MAX_RELCACHE_INVAL_MSGS) { - ListCell *lc; + foreach_oid(relid, relids) + { + HeapTuple tup; - foreach(lc, relids) - CacheInvalidateRelcacheByRelid(lfirst_oid(lc)); + tup = SearchSysCache1(RELOID, ObjectIdGetDatum(relid)); + if (!HeapTupleIsValid(tup)) + { + if (missing_ok) + return; + elog(ERROR, "cache lookup failed for relation %u", relid); + } + + CacheInvalidateRelcacheByTuple(tup); + ReleaseSysCache(tup); + } } else CacheInvalidateRelcacheAll(); @@ -1760,7 +1780,7 @@ RemovePublicationRelById(Oid proid) relids = GetPubPartitionOptionRelations(relids, PUBLICATION_PART_ALL, pubrel->prrelid); - InvalidatePublicationRels(relids); + InvalidatePublicationRels(relids, true); CatalogTupleDelete(rel, &tup->t_self); @@ -1825,7 +1845,7 @@ RemovePublicationSchemaById(Oid psoid) */ schemaRels = GetSchemaPublicationRelations(pubsch->pnnspid, PUBLICATION_PART_ALL); - InvalidatePublicationRels(schemaRels); + InvalidatePublicationRels(schemaRels, true); CatalogTupleDelete(rel, &tup->t_self); diff --git a/src/include/commands/publicationcmds.h b/src/include/commands/publicationcmds.h index 4cf45c17cc5..3fe55a61263 100644 --- a/src/include/commands/publicationcmds.h +++ b/src/include/commands/publicationcmds.h @@ -30,7 +30,7 @@ extern void RemovePublicationSchemaById(Oid psoid); extern ObjectAddress AlterPublicationOwner(const char *name, Oid newOwnerId); extern void AlterPublicationOwner_oid(Oid pubid, Oid newOwnerId); -extern void InvalidatePublicationRels(List *relids); +extern void InvalidatePublicationRels(List *relids, bool missing_ok); extern bool pub_rf_contains_invalid_column(Oid pubid, Relation relation, List *ancestors, bool pubviaroot); extern bool pub_contains_invalid_column(Oid pubid, Relation relation, -- 2.55.0