From cc7b4021d5661aa8dbb8bd3a8c8569824dbbbc36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= 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