From 6796a27c885276a4c19fadef2d757adff4acd178 Mon Sep 17 00:00:00 2001 From: Ajin Cherian Date: Thu, 12 Nov 2026 14:50:26 +1100 Subject: [PATCH v1] Fix cache lookup failure during concurrent DROP PUBLICATION pg_get_publication_tables() collects (pubid, relid) pairs on its first call and then looks each publication up again per row with GetPublication(). No lock is held on the publication, so if it is dropped concurrently -- for instance while the function waits for the lock in try_table_open(), which also processes invalidations -- the lookup fails with "cache lookup failed for publication". Reported-by: Noah Discussion: https://postgr.es/m/19864-5260a1fbeec7728c@postgresql.org --- src/backend/catalog/pg_publication.c | 33 ++++++++++++++++--- .../expected/pub-concurrent-drop.out | 14 ++++++++ .../isolation/specs/pub-concurrent-drop.spec | 11 +++++-- 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index f11ea8839ca..76771e22c23 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -1305,9 +1305,12 @@ GetAllSchemaPublicationRelations(Oid pubid, PublicationPartOpt pub_partopt) * Get publication using oid * * The Publication struct and its data are palloc'ed here. + * + * If missing_ok is true, return NULL when the publication does not exist + * (e.g., it was dropped concurrently); otherwise raise an error. */ -Publication * -GetPublication(Oid pubid) +static Publication * +get_publication_internal(Oid pubid, bool missing_ok) { HeapTuple tup; Publication *pub; @@ -1315,7 +1318,11 @@ GetPublication(Oid pubid) tup = SearchSysCache1(PUBLICATIONOID, ObjectIdGetDatum(pubid)); if (!HeapTupleIsValid(tup)) + { + if (missing_ok) + return NULL; elog(ERROR, "cache lookup failed for publication %u", pubid); + } pubform = (Form_pg_publication) GETSTRUCT(tup); @@ -1336,6 +1343,12 @@ GetPublication(Oid pubid) return pub; } +Publication * +GetPublication(Oid pubid) +{ + return get_publication_internal(pubid, false); +} + /* * Get Publication using name. */ @@ -1647,10 +1660,22 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames, continue; /* - * Form tuple with appropriate data. + * Likewise, the publication OIDs were collected earlier and we don't + * hold a lock on the publication, so it may have been dropped + * concurrently (for instance, while we waited for the lock on the + * table above, which also processes pending invalidations). Skip the + * row in that case, for the same point-in-time reasoning as above. */ + pub = get_publication_internal(table_info->pubid, true); + if (pub == NULL) + { + table_close(rel, AccessShareLock); + continue; + } - pub = GetPublication(table_info->pubid); + /* + * Form tuple with appropriate data. + */ values[0] = ObjectIdGetDatum(pub->oid); values[1] = ObjectIdGetDatum(relid); diff --git a/src/test/isolation/expected/pub-concurrent-drop.out b/src/test/isolation/expected/pub-concurrent-drop.out index 8360af0ec9c..28f8e630b70 100644 --- a/src/test/isolation/expected/pub-concurrent-drop.out +++ b/src/test/isolation/expected/pub-concurrent-drop.out @@ -14,3 +14,17 @@ tablename pubdrop.keepme (1 row) + +starting permutation: lock list_pub_tables drop_pub_and_commit +step lock: BEGIN; LOCK pubdrop.dropme IN ACCESS EXCLUSIVE MODE; +step list_pub_tables: + SELECT relid::regclass AS tablename + FROM pg_get_publication_tables('pub_schema') + ORDER BY tablename; + +step drop_pub_and_commit: DROP PUBLICATION pub_schema; COMMIT; +step list_pub_tables: <... completed> +tablename +--------- +(0 rows) + diff --git a/src/test/isolation/specs/pub-concurrent-drop.spec b/src/test/isolation/specs/pub-concurrent-drop.spec index 4f7d701d60c..2f52208750b 100644 --- a/src/test/isolation/specs/pub-concurrent-drop.spec +++ b/src/test/isolation/specs/pub-concurrent-drop.spec @@ -1,4 +1,4 @@ -# Tests for concurrently dropping a relation while a publication's tables are +# Tests for concurrently dropping a relation or the publication itself while a publication's tables are # being listed. setup @@ -12,12 +12,13 @@ setup teardown { DROP SCHEMA pubdrop CASCADE; - DROP PUBLICATION pub_schema; + DROP PUBLICATION IF EXISTS pub_schema; } session s1 step lock { BEGIN; LOCK pubdrop.dropme IN ACCESS EXCLUSIVE MODE; } step drop_and_commit { DROP TABLE pubdrop.dropme; COMMIT; } +step drop_pub_and_commit { DROP PUBLICATION pub_schema; COMMIT; } session s2 step list_pub_tables @@ -34,3 +35,9 @@ step list_pub_tables # skips the now-dropped table instead of erroring with "could not open relation # with OID". permutation lock list_pub_tables drop_and_commit + +# Same as above, but drop the publication instead of the table. When the query +# resumes, the publication it collected the table OIDs from is gone, so the +# rows for it are skipped instead of erroring with "cache lookup failed for +# publication". +permutation lock list_pub_tables drop_pub_and_commit -- 2.47.3