From 93159d53ebfb9cb4f1883761b1028dad2c509537 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Sat, 4 Jul 2026 03:09:41 +0000 Subject: [PATCH] Fix pg_get_publication_tables race with concurrent DROP TABLE The pg_get_publication_tables() SRF collects table OIDs in its first call, and then opens the tables using table_open() for FOR ALL TABLES publications. If a table is dropped in between, the table_open() errors out with "could not open relation with OID". This is common in environments where many tables are being created and dropped while publication tables are being queried. The bug was introduced by b7ae03953690 in PG16. This commit fixes it by using try_table_open(), which returns NULL instead of erroring out if the relation no longer exists. We now open all tables this way regardless of whether a column list is specified, so a concurrently dropped table is consistently skipped. Such tables are simply absent from the result set, which is the expected point-in-time behavior. To skip dropped tables without emitting a row, we track the current index into the table list ourselves rather than relying on funcctx->call_cntr. Author: Bharath Rupireddy Reviewed-by: Bertrand Drouvot Reviewed-by: shveta malik Reviewed-by: Ajin Cherian Reviewed-by: Masahiko Sawada Discussion: https://www.postgresql.org/message-id/CALj2ACVYYooWH-5tJ6cPKkU%2BmutVxwb_z4S%2BqAi-zdrFqxXE2Q%40mail.gmail.com Backpatch-through: 16 --- src/backend/catalog/pg_publication.c | 52 +++++++++++++++---- .../expected/pub-concurrent-drop.out | 16 ++++++ src/test/isolation/isolation_schedule | 1 + .../isolation/specs/pub-concurrent-drop.spec | 33 ++++++++++++ src/tools/pgindent/typedefs.list | 1 + 5 files changed, 94 insertions(+), 9 deletions(-) create mode 100644 src/test/isolation/expected/pub-concurrent-drop.out create mode 100644 src/test/isolation/specs/pub-concurrent-drop.spec diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index 0602398a545..0b21076dbc4 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -1052,14 +1052,27 @@ Datum pg_get_publication_tables(PG_FUNCTION_ARGS) { #define NUM_PUBLICATION_TABLES_ELEM 4 + + /* + * State carried across SRF calls. We track the index ourselves instead of + * using funcctx->call_cntr, so that concurrently dropped tables can be + * skipped without emitting a row. + */ + typedef struct + { + List *table_infos; /* list of published_rel */ + int curr_idx; /* current index into table_infos */ + } publication_tables_state; + FuncCallContext *funcctx; - List *table_infos = NIL; + publication_tables_state *ptstate = NULL; /* stuff done only on the first call of the function */ if (SRF_IS_FIRSTCALL()) { TupleDesc tupdesc; MemoryContext oldcontext; + List *table_infos = NIL; ArrayType *arr; Datum *elems; int nelems, @@ -1158,26 +1171,47 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) PG_NODE_TREEOID, -1, 0); funcctx->tuple_desc = BlessTupleDesc(tupdesc); - funcctx->user_fctx = (void *) table_infos; + + /* Store the state to be used across SRF calls. */ + ptstate = palloc_object(publication_tables_state); + ptstate->table_infos = table_infos; + ptstate->curr_idx = 0; + funcctx->user_fctx = ptstate; MemoryContextSwitchTo(oldcontext); } /* stuff done on every call of the function */ funcctx = SRF_PERCALL_SETUP(); - table_infos = (List *) funcctx->user_fctx; + ptstate = (publication_tables_state *) funcctx->user_fctx; - if (funcctx->call_cntr < list_length(table_infos)) + while (ptstate->curr_idx < list_length(ptstate->table_infos)) { HeapTuple pubtuple = NULL; HeapTuple rettuple; Publication *pub; - published_rel *table_info = (published_rel *) list_nth(table_infos, funcctx->call_cntr); + published_rel *table_info = (published_rel *) list_nth(ptstate->table_infos, + ptstate->curr_idx); Oid relid = table_info->relid; - Oid schemaid = get_rel_namespace(relid); + Relation rel; + Oid schemaid; Datum values[NUM_PUBLICATION_TABLES_ELEM] = {0}; bool nulls[NUM_PUBLICATION_TABLES_ELEM] = {0}; + /* Advance the index for the next call. */ + ptstate->curr_idx++; + + /* + * The table OIDs were collected earlier, so a table may have been + * dropped before we get here. try_table_open() returns NULL if it is + * already gone, in which case we skip it; such tables are simply + * absent from the result set, which is the expected point-in-time + * behavior. + */ + rel = try_table_open(relid, AccessShareLock); + if (rel == NULL) + continue; + /* * Form tuple with appropriate data. */ @@ -1191,6 +1225,7 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) * We don't consider row filters or column lists for FOR ALL TABLES or * FOR TABLES IN SCHEMA publications. */ + schemaid = RelationGetNamespace(rel); if (!pub->alltables && !SearchSysCacheExists2(PUBLICATIONNAMESPACEMAP, ObjectIdGetDatum(schemaid), @@ -1220,7 +1255,6 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) /* Show all columns when the column list is not specified. */ if (nulls[2]) { - Relation rel = table_open(relid, AccessShareLock); int nattnums = 0; int16 *attnums; TupleDesc desc = RelationGetDescr(rel); @@ -1243,10 +1277,10 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) values[2] = PointerGetDatum(buildint2vector(attnums, nattnums)); nulls[2] = false; } - - table_close(rel, AccessShareLock); } + table_close(rel, AccessShareLock); + rettuple = heap_form_tuple(funcctx->tuple_desc, values, nulls); SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(rettuple)); diff --git a/src/test/isolation/expected/pub-concurrent-drop.out b/src/test/isolation/expected/pub-concurrent-drop.out new file mode 100644 index 00000000000..4fd16b1d0ca --- /dev/null +++ b/src/test/isolation/expected/pub-concurrent-drop.out @@ -0,0 +1,16 @@ +Parsed test spec with 2 sessions + +starting permutation: lock query drop_and_commit +step lock: BEGIN; LOCK pubdrop.dropme IN ACCESS EXCLUSIVE MODE; +step query: + SELECT tablename FROM pg_publication_tables + WHERE pubname = 'pub_all' AND schemaname = 'pubdrop' + ORDER BY tablename; + +step drop_and_commit: DROP TABLE pubdrop.dropme; COMMIT; +step query: <... completed> +tablename +--------- +keepme +(1 row) + diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 7a4ab2ce805..06efed5279b 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -94,6 +94,7 @@ test: async-notify test: vacuum-no-cleanup-lock test: timeouts test: vacuum-concurrent-drop +test: pub-concurrent-drop test: vacuum-conflict test: vacuum-skip-locked test: stats diff --git a/src/test/isolation/specs/pub-concurrent-drop.spec b/src/test/isolation/specs/pub-concurrent-drop.spec new file mode 100644 index 00000000000..aabe3812f3c --- /dev/null +++ b/src/test/isolation/specs/pub-concurrent-drop.spec @@ -0,0 +1,33 @@ +# BUG: pg_get_publication_tables() errors with "could not open relation with +# OID" when a table is dropped concurrently. + +setup +{ + CREATE SCHEMA pubdrop; + CREATE PUBLICATION pub_all FOR ALL TABLES; + CREATE TABLE pubdrop.dropme (id int); + CREATE TABLE pubdrop.keepme (id int); +} + +teardown +{ + DROP SCHEMA pubdrop CASCADE; + DROP PUBLICATION pub_all; +} + +session s1 +# Hold an ACCESS EXCLUSIVE lock on the table in a separate session, so that +# the pg_publication_tables query will block when it tries to open the table. +step lock { BEGIN; LOCK pubdrop.dropme IN ACCESS EXCLUSIVE MODE; } +# Drop the table in the lock-holding session and commit, releasing the lock. +step drop_and_commit { DROP TABLE pubdrop.dropme; COMMIT; } + +session s2 +step query +{ + SELECT tablename FROM pg_publication_tables + WHERE pubname = 'pub_all' AND schemaname = 'pubdrop' + ORDER BY tablename; +} + +permutation lock query drop_and_commit diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index e5634ae2969..86b194da8e3 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3805,6 +3805,7 @@ pthread_mutex_t pthread_once_t pthread_t ptrdiff_t +publication_tables_state published_rel pull_var_clause_context pull_varattnos_context -- 2.47.3