v7-0001-Fix-pg_get_publication_tables-race-with-concurren.patch
application/x-patch
Filename: v7-0001-Fix-pg_get_publication_tables-race-with-concurren.patch
Type: application/x-patch
Part: 0
Patch
Same data as JSON:
GET /api/v1/attachments/:id/patch
the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes.
API reference →
Format: format-patch
Series: patch v7-0001
Subject: Fix pg_get_publication_tables race with concurrent DROP TABLE
| File | + | − |
|---|---|---|
| src/backend/catalog/pg_publication.c | 101 | 133 |
| src/test/subscription/t/100_bugs.pl | 46 | 0 |
From a3a3ff5c291043abdbb5123ca092e9c778f8b934 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Mon, 15 Jun 2026 22:43:14 +0000
Subject: [PATCH v7 1/2] Fix pg_get_publication_tables race with concurrent
DROP TABLE
pg_get_publication_tables() collects table OIDs first, and then
opens all the tables, for which column lists are not specified,
using table_open(). If a table is dropped in between, the
table_open() errors with
"could not open relation with OID". This is common in environments
where many tables are being created and dropped while
pg_publication_tables view is queried, such as with
FOR ALL TABLES publications.
The bug was introduced by b7ae03953690 in PG16.
This commit fixes it by using try_table_open() which returns NULL
insetad of erroring out if the relation does not exist. Tables
created after the list is built are simply not present
in the result set, which is expected point-in-time behavior.
On HEAD, use a tuplestore-based SRF instead of the traditional
SRF per-call mechanism to simplify the code and avoid introducing
additional structures.
On pre-HEAD versions, define a new struct to carry the current
index into the tables list across multiple invocations, to help
correctly skip tables dropped concurrently. This is to keep
code simple in the backpatch branches.
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: shveta malik <shveta.malik@gmail.com>
Reviewed-by: Ajin Cherian <itsajin@gmail.com>
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 | 234 ++++++++++++---------------
src/test/subscription/t/100_bugs.pl | 46 ++++++
2 files changed, 147 insertions(+), 133 deletions(-)
diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index 5c457d9aca8..164975cd0cb 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -37,6 +37,7 @@
#include "utils/catcache.h"
#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
+#include "utils/tuplestore.h"
#include "utils/rel.h"
#include "utils/syscache.h"
@@ -1414,160 +1415,123 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames,
bool pub_missing_ok)
{
#define NUM_PUBLICATION_TABLES_ELEM 4
- FuncCallContext *funcctx;
+ ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
List *table_infos = NIL;
+ Datum *elems;
+ int nelems,
+ i;
+ bool viaroot = false;
+ ListCell *lc;
- /* stuff done only on the first call of the function */
- if (SRF_IS_FIRSTCALL())
- {
- TupleDesc tupdesc;
- MemoryContext oldcontext;
- Datum *elems;
- int nelems,
- i;
- bool viaroot = false;
-
- /* create a function context for cross-call persistence */
- funcctx = SRF_FIRSTCALL_INIT();
-
- /*
- * Preliminary check if the specified table can be published in the
- * first place. If not, we can return early without checking the given
- * publications and the table.
- */
- if (filter_by_relid && !is_publishable_table(target_relid))
- SRF_RETURN_DONE(funcctx);
+ InitMaterializedSRF(fcinfo, 0);
- /* switch to memory context appropriate for multiple function calls */
- oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+ /*
+ * Preliminary check if the specified table can be published in the first
+ * place. If not, we can return early without checking the given
+ * publications and the table.
+ */
+ if (filter_by_relid && !is_publishable_table(target_relid))
+ return (Datum) 0;
- /*
- * Deconstruct the parameter into elements where each element is a
- * publication name.
- */
- deconstruct_array_builtin(pubnames, TEXTOID, &elems, NULL, &nelems);
+ /*
+ * Deconstruct the parameter into elements where each element is a
+ * publication name.
+ */
+ deconstruct_array_builtin(pubnames, TEXTOID, &elems, NULL, &nelems);
- /* Get Oids of tables from each publication. */
- for (i = 0; i < nelems; i++)
- {
- Publication *pub_elem;
- List *pub_elem_tables = NIL;
- ListCell *lc;
+ /* Get Oids of tables from each publication. */
+ for (i = 0; i < nelems; i++)
+ {
+ Publication *pub_elem;
+ List *pub_elem_tables = NIL;
+ ListCell *lc2;
- pub_elem = GetPublicationByName(TextDatumGetCString(elems[i]),
- pub_missing_ok);
+ pub_elem = GetPublicationByName(TextDatumGetCString(elems[i]),
+ pub_missing_ok);
- if (pub_elem == NULL)
- continue;
+ if (pub_elem == NULL)
+ continue;
- if (filter_by_relid)
- {
- /* Check if the given table is published for the publication */
- if (is_table_publishable_in_publication(target_relid, pub_elem))
- {
- pub_elem_tables = list_make1_oid(target_relid);
- }
- }
- else
+ if (filter_by_relid)
+ {
+ /* Check if the given table is published for the publication */
+ if (is_table_publishable_in_publication(target_relid, pub_elem))
{
- /*
- * Publications support partitioned tables. If
- * publish_via_partition_root is false, all changes are
- * replicated using leaf partition identity and schema, so we
- * only need those. Otherwise, get the partitioned table
- * itself.
- */
- if (pub_elem->alltables)
- pub_elem_tables = GetAllPublicationRelations(pub_elem->oid,
- RELKIND_RELATION,
- pub_elem->pubviaroot);
- else
- {
- List *relids,
- *schemarelids;
-
- relids = GetIncludedPublicationRelations(pub_elem->oid,
- pub_elem->pubviaroot ?
- PUBLICATION_PART_ROOT :
- PUBLICATION_PART_LEAF);
- schemarelids = GetAllSchemaPublicationRelations(pub_elem->oid,
- pub_elem->pubviaroot ?
- PUBLICATION_PART_ROOT :
- PUBLICATION_PART_LEAF);
- pub_elem_tables = list_concat_unique_oid(relids, schemarelids);
- }
+ pub_elem_tables = list_make1_oid(target_relid);
}
-
+ }
+ else
+ {
/*
- * Record the published table and the corresponding publication so
- * that we can get row filters and column lists later.
- *
- * When a table is published by multiple publications, to obtain
- * all row filters and column lists, the structure related to this
- * table will be recorded multiple times.
+ * Publications support partitioned tables. If
+ * publish_via_partition_root is false, all changes are replicated
+ * using leaf partition identity and schema, so we only need
+ * those. Otherwise, get the partitioned table itself.
*/
- foreach(lc, pub_elem_tables)
+ if (pub_elem->alltables)
+ pub_elem_tables = GetAllPublicationRelations(pub_elem->oid,
+ RELKIND_RELATION,
+ pub_elem->pubviaroot);
+ else
{
- published_rel *table_info = palloc_object(published_rel);
-
- table_info->relid = lfirst_oid(lc);
- table_info->pubid = pub_elem->oid;
- table_infos = lappend(table_infos, table_info);
+ List *relids,
+ *schemarelids;
+
+ relids = GetIncludedPublicationRelations(pub_elem->oid,
+ pub_elem->pubviaroot ?
+ PUBLICATION_PART_ROOT :
+ PUBLICATION_PART_LEAF);
+ schemarelids = GetAllSchemaPublicationRelations(pub_elem->oid,
+ pub_elem->pubviaroot ?
+ PUBLICATION_PART_ROOT :
+ PUBLICATION_PART_LEAF);
+ pub_elem_tables = list_concat_unique_oid(relids, schemarelids);
}
-
- /* At least one publication is using publish_via_partition_root. */
- if (pub_elem->pubviaroot)
- viaroot = true;
}
/*
- * If the publication publishes partition changes via their respective
- * root partitioned tables, we must exclude partitions in favor of
- * including the root partitioned tables. Otherwise, the function
- * could return both the child and parent tables which could cause
- * data of the child table to be double-published on the subscriber
- * side.
+ * Record the published table and the corresponding publication so
+ * that we can get row filters and column lists later.
+ *
+ * When a table is published by multiple publications, to obtain all
+ * row filters and column lists, the structure related to this table
+ * will be recorded multiple times.
*/
- if (viaroot)
- filter_partitions(table_infos);
-
- /* Construct a tuple descriptor for the result rows. */
- tupdesc = CreateTemplateTupleDesc(NUM_PUBLICATION_TABLES_ELEM);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "pubid",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 2, "relid",
- OIDOID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 3, "attrs",
- INT2VECTOROID, -1, 0);
- TupleDescInitEntry(tupdesc, (AttrNumber) 4, "qual",
- PG_NODE_TREEOID, -1, 0);
-
- TupleDescFinalize(tupdesc);
- funcctx->tuple_desc = BlessTupleDesc(tupdesc);
- funcctx->user_fctx = table_infos;
+ foreach(lc2, pub_elem_tables)
+ {
+ published_rel *table_info = palloc_object(published_rel);
- MemoryContextSwitchTo(oldcontext);
+ table_info->relid = lfirst_oid(lc2);
+ table_info->pubid = pub_elem->oid;
+ table_infos = lappend(table_infos, table_info);
+ }
+
+ /* At least one publication is using publish_via_partition_root. */
+ if (pub_elem->pubviaroot)
+ viaroot = true;
}
- /* stuff done on every call of the function */
- funcctx = SRF_PERCALL_SETUP();
- table_infos = (List *) funcctx->user_fctx;
+ /*
+ * If the publication publishes partition changes via their respective
+ * root partitioned tables, we must exclude partitions in favor of
+ * including the root partitioned tables. Otherwise, the function could
+ * return both the child and parent tables which could cause data of the
+ * child table to be double-published on the subscriber side.
+ */
+ if (viaroot)
+ filter_partitions(table_infos);
- if (funcctx->call_cntr < list_length(table_infos))
+ /* Produce a result row for each published table. */
+ foreach(lc, 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 *) lfirst(lc);
Oid relid = table_info->relid;
+ Publication *pub;
+ HeapTuple pubtuple = NULL;
Oid schemaid = get_rel_namespace(relid);
Datum values[NUM_PUBLICATION_TABLES_ELEM] = {0};
bool nulls[NUM_PUBLICATION_TABLES_ELEM] = {0};
- /*
- * Form tuple with appropriate data.
- */
-
pub = GetPublication(table_info->pubid);
values[0] = ObjectIdGetDatum(pub->oid);
@@ -1606,11 +1570,16 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames,
/* Show all columns when the column list is not specified. */
if (nulls[2])
{
- Relation rel = table_open(relid, AccessShareLock);
+ Relation rel = try_table_open(relid, AccessShareLock);
int nattnums = 0;
int16 *attnums;
- TupleDesc desc = RelationGetDescr(rel);
- int i;
+ TupleDesc desc;
+
+ /* Skip if the relation has been concurrently dropped. */
+ if (rel == NULL)
+ continue;
+
+ desc = RelationGetDescr(rel);
attnums = palloc_array(int16, desc->natts);
@@ -1647,12 +1616,11 @@ pg_get_publication_tables(FunctionCallInfo fcinfo, ArrayType *pubnames,
table_close(rel, AccessShareLock);
}
- rettuple = heap_form_tuple(funcctx->tuple_desc, values, nulls);
-
- SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(rettuple));
+ tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc,
+ values, nulls);
}
- SRF_RETURN_DONE(funcctx);
+ return (Datum) 0;
}
Datum
diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index a23035e23fe..5ff4098434d 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -605,4 +605,50 @@ $node_publisher->safe_psql('postgres', "DROP DATABASE regress_db");
$node_publisher->stop('fast');
+# BUG: pg_get_publication_tables() errors with "could not open relation with
+# OID" when a table is dropped concurrently.
+$node_publisher->start();
+
+$node_publisher->safe_psql(
+ 'postgres', qq{
+ CREATE PUBLICATION pub_all FOR ALL TABLES;
+ CREATE TABLE t_dropme (id int, data text);
+});
+
+# 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.
+my $holder = $node_publisher->background_psql('postgres');
+$holder->query_safe("BEGIN; LOCK TABLE t_dropme IN ACCESS EXCLUSIVE MODE;");
+
+# Background session queries pg_publication_tables; it will block waiting for
+# the lock on the table.
+my $bgpsql =
+ $node_publisher->background_psql('postgres', on_error_stop => 0);
+$bgpsql->query_until(
+ qr/querying_publication_tables/,
+ qq{\\echo querying_publication_tables
+SELECT count(*) FROM pg_publication_tables WHERE pubname = 'pub_all';
+});
+
+# Wait until the querying session is blocked on the lock.
+$node_publisher->poll_query_until('postgres',
+ "SELECT count(*) > 0 FROM pg_stat_activity WHERE wait_event_type = 'Lock' AND query LIKE '%pg_publication_tables%';"
+);
+
+# Drop the table in the lock-holding session and commit, releasing the lock.
+$holder->query_safe("DROP TABLE t_dropme; COMMIT;");
+$holder->quit;
+
+# Verify the background session completed without error.
+my $bg_result = $bgpsql->query_safe("SELECT 1");
+$bgpsql->quit;
+
+ok(defined($bg_result),
+ "pg_publication_tables handles concurrently dropped tables");
+
+# Cleanup.
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION pub_all");
+
+$node_publisher->stop('fast');
+
done_testing();
--
2.47.3