Here's the additional changes I made here before giving up on this.
I think it needs some additional rethinking, not going to happen for 18.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"The Gord often wonders why people threaten never to come back after they've
been told never to return" (www.actsofgord.com)
>From cc7b4021d5661aa8dbb8bd3a8c8569824dbbbc36 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvhe...@alvh.no-ip.org>
Date: Mon, 7 Apr 2025 14:31:44 +0200
Subject: [PATCH] some more changes
---
src/backend/catalog/pg_publication.c | 63 ++++++++++++-----------
src/test/regress/expected/publication.out | 16 ++++++
src/test/regress/sql/publication.sql | 10 ++++
3 files changed, 59 insertions(+), 30 deletions(-)
diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index 51e463c112b..ec76c0c8eb2 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -34,6 +34,7 @@
#include "commands/publicationcmds.h"
#include "funcapi.h"
#include "partitioning/partdesc.h"
+#include "storage/lmgr.h"
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/catcache.h"
@@ -130,53 +131,56 @@ check_publication_add_schema(Oid schemaid)
/*
* publication_check_foreign_parts
- * Helper function to ensure we don't publish foreign tables
+ * Helper function to ensure we don't indirectly publish foreign tables
*
- * Foreign tables may not be published.
+ * DML data changes are not published for data in foreign tables,
+ * and yet the tablesync worker is not smart enough to omit data from
+ * foreign tables when they are partitions of partitioned tables. To
+ * avoid the inconsistencies that would result, we disallow foreign
+ * tables from being published generally. However, it's possible for
+ * partitioned tables to have foreign tables as partitions, and we would
+ * like to allow publishing those partitioned tables so that the other
+ * partitions are replicated.
*
- * Protect against including foreign tables that are partitions of partitioned
- * tables published by the given publication when publish_via_root_partition is
- * true. This will not work correctly as the initial data from the foreign
- * table can be replicated by the tablesync worker even though replication of
- * foreign table is not supported because when publish_via_root_partition is
- * true, the root table is included in pg_subscription_rel catalog table and
- * tablesync worker cannot distinguish data from foreign partition. So we
- * disallow the case here and in all DDL commands that would end up creating
- * such a case indirectly.
+ * This function is in charge of detecting if a partitioned table has a
+ * foreign table as a partition -- either in the whole database (useful
+ * for FOR ALL TABLES publications) or in a particular schema (useful
+ * for FOR TABLES IN SCHEMA publications). This function must be called
+ * only for publications with publish_via_partition_root=true.
*
- * When publish_via_root_partition is set to false, leaf partitions are included
- * in pg_subscription_rel catalog table. So, when we include a partition table
- * with foreign partition in a publication, we skip including foreign partitions
- * to pg_subscription_rel catalog table. So, the foreign partitions are not
- * replicated.
+ * When publish_via_partition_root is false, each partition published for
+ * replication is listed individually in pg_replication_rel, and we
+ * don't add partitions that are foreign tables, so this check is not
+ * needed.
*
+ * If a valid schemaid is provided, check if that schema has any
+ * partitioned table with a foreign table as partition.
*
+ * If no valid schemaid is provided, check all partitioned tables.
*
- * If valid schemaid provided, check if the schema has a partition table which
- * has a foreign partition. The partition tables in a schema can have partitions
- * in other schema. We also need to check if such partitions are foreign
- * partition.
+ * We take a lock on partition tables so no new foreign partitions are
+ * added concurrently.
*
- * If valid schemaid is not provided, we get all partition tables and check if
- * it has any foreign partition. We take a lock on partition tables so no new
- * foreign partitions are added concurrently.
- *
- * We take a ShareLock on pg_partitioned_table to restrict addition of new
- * partitioned table which may contain a foreign partition while publication is
- * being created.
+ * We also take a ShareLock on pg_partitioned_table to restrict addition
+ * of new partitioned table which may contain a foreign partition while
+ * publication is being created. XXX this is quite weird actually.
*/
void
publication_check_foreign_parts(Oid schemaid, char *pubname)
{
Relation classRel;
- Relation partRel;
ScanKeyData key[3];
int keycount = 0;
TableScanDesc scan;
HeapTuple tuple;
+ /*
+ * Take lock on pg_partitioned_rel. This prevents new publications
+ * from being created.
+ */
+ LockRelationOid(PartitionedRelationId, ShareLock);
+
classRel = table_open(RelationRelationId, AccessShareLock);
- partRel = table_open(PartitionedRelationId, ShareLock);
/* Get the root nodes of partitioned table */
ScanKeyInit(&key[keycount++],
@@ -222,7 +226,6 @@ publication_check_foreign_parts(Oid schemaid, char *pubname)
table_endscan(scan);
table_close(classRel, AccessShareLock);
- table_close(partRel, NoLock);
}
/*
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 4395563fcd5..678f8b52e5b 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -2006,6 +2006,22 @@ SELECT pubname, tablename FROM pg_publication_tables WHERE schemaname in ('sch3'
ALTER PUBLICATION pub1 SET (publish_via_partition_root);
ERROR: cannot set parameter "publish_via_partition_root" to "true" for publication "pub1"
DETAIL: Published partitioned table "tmain" contains a partition that is a foreign table.
+CREATE SCHEMA sch5;
+CREATE SCHEMA sch6;
+CREATE TABLE sch6.tmain(id int) PARTITION BY RANGE (id);
+CREATE TABLE sch5.part1 PARTITION OF sch6.tmain FOR VALUES FROM (0) TO (10) PARTITION BY RANGE(id);
+CREATE FOREIGN TABLE sch6.part2 PARTITION OF sch5.part1 FOR VALUES FROM (0) TO (5) SERVER fdw_server;
+CREATE PUBLICATION pub4 FOR TABLES IN SCHEMA sch5
+ WITH (publish_via_partition_root); -- should fail
+ERROR: cannot add table ...
+SELECT pubname, tablename FROM pg_publication_tables WHERE pubname = 'pub4' ORDER BY pubname, tablename;
+ pubname | tablename
+---------+-----------
+ pub4 | part1
+(1 row)
+
+DROP SCHEMA sch5, sch6 CASCADE;
+DROP PUBLICATION pub4;
DROP PUBLICATION pub1;
DROP PUBLICATION pub2;
DROP PUBLICATION pub3;
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index 49c9d98b668..43755b973cb 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -1296,6 +1296,16 @@ SELECT pubname, tablename FROM pg_publication_tables WHERE schemaname in ('sch3'
-- foreign partition
ALTER PUBLICATION pub1 SET (publish_via_partition_root);
+CREATE SCHEMA sch5;
+CREATE SCHEMA sch6;
+CREATE TABLE sch6.tmain(id int) PARTITION BY RANGE (id);
+CREATE TABLE sch5.part1 PARTITION OF sch6.tmain FOR VALUES FROM (0) TO (10) PARTITION BY RANGE(id);
+CREATE FOREIGN TABLE sch6.part2 PARTITION OF sch5.part1 FOR VALUES FROM (0) TO (5) SERVER fdw_server;
+CREATE PUBLICATION pub4 FOR TABLES IN SCHEMA sch5 WITH (publish_via_partition_root);
+SELECT pubname, tablename FROM pg_publication_tables WHERE pubname = 'pub4' ORDER BY pubname, tablename;
+DROP SCHEMA sch5, sch6 CASCADE;
+DROP PUBLICATION pub4;
+
DROP PUBLICATION pub1;
DROP PUBLICATION pub2;
DROP PUBLICATION pub3;
--
2.39.5
