On 2025-Apr-01, Shlok Kyal wrote:

> I have modified the comment in create_publication.sgml and also added
> comment in the restrictions section of logical-replication.sgml.
> I have also added a more detailed explanation in comment of
> 'check_foreign_tables'
> 
> I have attached the updated v11 patch.

Sadly I don't have time to describe the changes proposed here right now,
but I'll do that early tomorrow.  (Some minor changes are still needed,
particularly the comments to publication_check_foreign_parts which are
mostly unchanged from what your patch has.  I'll do another review round
tomorrow.)

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
>From c6433548f3f47b23b81384f3cc80c0df307f6415 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvhe...@alvh.no-ip.org>
Date: Sun, 6 Apr 2025 21:27:39 +0200
Subject: [PATCH] some changes

---
 doc/src/sgml/logical-replication.sgml     |  18 +-
 doc/src/sgml/ref/create_publication.sgml  |  24 ++-
 src/backend/catalog/pg_publication.c      | 235 ++++++++++------------
 src/backend/commands/foreigncmds.c        |  49 -----
 src/backend/commands/publicationcmds.c    |  42 ++--
 src/backend/commands/tablecmds.c          | 109 +++++++---
 src/backend/partitioning/partdesc.c       |  32 +++
 src/include/catalog/pg_publication.h      |   6 +-
 src/include/partitioning/partdesc.h       |   1 +
 src/test/regress/expected/publication.out |  30 +--
 10 files changed, 287 insertions(+), 259 deletions(-)

diff --git a/doc/src/sgml/logical-replication.sgml 
b/doc/src/sgml/logical-replication.sgml
index ce60a1b391c..67503aec871 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -2154,18 +2154,18 @@ CONTEXT:  processing remote data for replication origin 
"pg_16395" during "INSER
 
    <listitem>
     <para>
-     Replication is supported only for tables, including partitioned tables,
-     except when they contain foreign partitions. If
-     <literal>publish_via_partition_root</literal> is set to
-     <literal>true</literal> for a publication table with foreign partitions, 
or
-     if an attempt is made to replicate such a table, an error is thrown.
-     Additionally, when replicating a partitioned table where
-     <literal>publish_via_partition_root</literal> is set to
-     <literal>false</literal> and foreign partitions are present, all 
partitions
-     are replicated except the foreign partitions.
+     Replication is only supported for tables, including partitioned tables.
      Attempts to replicate other types of relations, such as views, 
materialized
      views, or foreign tables, will result in an error.
     </para>
+    <para>
+     Replication is not supported for foreign tables.  When used as partitions
+     of partitioned tables, publishing of the partitioned table is only allowed
+     if the <literal>publish_via_partition_root</literal> is set to
+     <literal>false</literal>.  In this mode, changes to foreign partitions are
+     ignored for the purposes of replication, and data contained in them
+     is not included during initial synchronization.
+    </para>
    </listitem>
 
    <listitem>
diff --git a/doc/src/sgml/ref/create_publication.sgml 
b/doc/src/sgml/ref/create_publication.sgml
index 47216fc4789..9f412e6693a 100644
--- a/doc/src/sgml/ref/create_publication.sgml
+++ b/doc/src/sgml/ref/create_publication.sgml
@@ -248,20 +248,26 @@ CREATE PUBLICATION <replaceable 
class="parameter">name</replaceable>
           that of the individual partitions.
          </para>
 
+         <para>
+          If this parameter is enabled, <literal>TRUNCATE</literal>
+          operations performed directly on partitions are not replicated.
+         </para>
+
+         <para>
+          If this parameter is enabled, foreign tables and partitioned tables
+          containing partitions that are foreign tables may not be
+          added to the publication.  Conversely, foreign tables may not be
+          attached to a partitioned table that is included in a publication
+          with this parameter enabled.  Lastly, this parameter may not be
+          changed on publications that include partitioned tables with foreign
+          tables as partitions.
+         </para>
+
          <para>
           This parameter also affects how row filters and column lists are
           chosen for partitions; see below for details.
          </para>
 
-         <para>
-          If this is enabled, <literal>TRUNCATE</literal> operations performed
-          directly on partitions are not replicated.
-         </para>
-
-         <para>
-          If this is enabled, a foreign table or a partitioned table with a
-          foreign partition is not allowed in the publication.
-         </para>
         </listitem>
        </varlistentry>
       </variablelist></para>
diff --git a/src/backend/catalog/pg_publication.c 
b/src/backend/catalog/pg_publication.c
index 77299b75cde..51e463c112b 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -55,7 +55,7 @@ typedef struct
  * error if not.
  */
 static void
-check_publication_add_relation(Relation targetrel, Publication *pub)
+check_publication_add_relation(Publication *pub, Relation targetrel)
 {
        /* Must be a regular or partitioned table */
        if (RelationGetForm(targetrel)->relkind != RELKIND_RELATION &&
@@ -68,15 +68,18 @@ check_publication_add_relation(Relation targetrel, 
Publication *pub)
 
        /*
         * publish_via_root_partition cannot be true if it is a partitioned 
table
-        * and has any foreign partition. See check_foreign_tables for details.
+        * and has any foreign partition. See publication_check_foreign_parts 
for
+        * details.
         */
-       if (pub->pubviaroot && check_partrel_has_foreign_table(targetrel))
+       if (pub->pubviaroot &&
+               targetrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
+               RelationHasForeignPartition(targetrel))
                ereport(ERROR,
-                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                                errmsg("cannot set parameter \"%s\" to true 
for publication \"%s\"",
-                                               "publish_via_partition_root", 
pub->name),
-                                errdetail("partition table \"%s\" in 
publication contains a foreign partition",
-                                                  
RelationGetRelationName(targetrel))));
+                               errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                               errmsg("cannot add partitioned table \"%s\" to 
publication \"%s\", which has \"%s\" set to \"%s\"",
+                                          RelationGetRelationName(targetrel), 
pub->name,
+                                          "publish_via_partition_root", 
"true"),
+                               errdetail("The table contains a partition 
that's a foreign table."));
 
        /* Can't be system table */
        if (IsCatalogRelation(targetrel))
@@ -125,6 +128,103 @@ check_publication_add_schema(Oid schemaid)
                                 errdetail("Temporary schemas cannot be 
replicated.")));
 }
 
+/*
+ * publication_check_foreign_parts
+ *             Helper function to ensure we don't publish foreign tables
+ *
+ * Foreign tables may not be published.
+ *
+ * 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.
+ *
+ * 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.
+ *
+ *
+ *
+ * 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.
+ *
+ * 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.
+ */
+void
+publication_check_foreign_parts(Oid schemaid, char *pubname)
+{
+       Relation        classRel;
+       Relation        partRel;
+       ScanKeyData key[3];
+       int                     keycount = 0;
+       TableScanDesc scan;
+       HeapTuple       tuple;
+
+       classRel = table_open(RelationRelationId, AccessShareLock);
+       partRel = table_open(PartitionedRelationId, ShareLock);
+
+       /* Get the root nodes of partitioned table */
+       ScanKeyInit(&key[keycount++],
+                               Anum_pg_class_relkind,
+                               BTEqualStrategyNumber, F_CHAREQ,
+                               CharGetDatum(RELKIND_PARTITIONED_TABLE));
+
+       ScanKeyInit(&key[keycount++],
+                               Anum_pg_class_relispartition,
+                               BTEqualStrategyNumber, F_BOOLEQ,
+                               BoolGetDatum(false));
+
+       /* If schema id is provided check partitioned table in that schema */
+       if (OidIsValid(schemaid))
+               ScanKeyInit(&key[keycount++],
+                                       Anum_pg_class_relnamespace,
+                                       BTEqualStrategyNumber, F_OIDEQ,
+                                       schemaid);
+
+       scan = table_beginscan_catalog(classRel, keycount, key);
+       while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
+       {
+               Form_pg_class relForm = (Form_pg_class) GETSTRUCT(tuple);
+               Relation        pubrel = table_open(relForm->oid, 
AccessShareLock);
+
+               if (pubrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
+                       RelationHasForeignPartition(pubrel))
+                       ereport(ERROR,
+                                       
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                        errmsg("cannot set parameter \"%s\" to 
\"%s\" for publication \"%s\"",
+                                                       
"publish_via_partition_root", "true", pubname),
+                                        errtable(pubrel),
+                                        errdetail("Published partitioned table 
\"%s\" contains a partition that is a foreign table.",
+                                                          
get_rel_name(relForm->oid))));
+
+               /*
+                * Keep lock till end of transaction: must prevent this table 
from
+                * being attached a foreign table until we're done.  XXX does 
this
+                * prevent addition of a partition in a partitioned child?
+                */
+               table_close(pubrel, NoLock);
+       }
+
+       table_endscan(scan);
+       table_close(classRel, AccessShareLock);
+       table_close(partRel, NoLock);
+}
+
 /*
  * Returns if relation represented by oid and Form_pg_class entry
  * is publishable.
@@ -473,7 +573,7 @@ publication_add_relation(Oid pubid, PublicationRelInfo *pri,
                                                
RelationGetRelationName(targetrel), pub->name)));
        }
 
-       check_publication_add_relation(targetrel, pub);
+       check_publication_add_relation(pub, targetrel);
 
        /* Validate and translate column names into a Bitmapset of attnums. */
        attnums = pub_collist_validate(pri->relation, pri->columns);
@@ -718,7 +818,7 @@ publication_add_schema(Oid pubid, Oid schemaid, bool 
if_not_exists)
         * partition
         */
        if (pub->pubviaroot)
-               check_foreign_tables(schemaid, pub->name);
+               publication_check_foreign_parts(schemaid, pub->name);
 
        /* Form a tuple */
        memset(values, 0, sizeof(values));
@@ -1349,118 +1449,3 @@ pg_get_publication_tables(PG_FUNCTION_ARGS)
 
        SRF_RETURN_DONE(funcctx);
 }
-
-/* Check if a partitioned table has a foreign partition */
-bool
-check_partrel_has_foreign_table(Relation rel)
-{
-       bool            has_foreign_tbl = false;
-
-       if (RelationGetForm(rel)->relkind == RELKIND_PARTITIONED_TABLE)
-       {
-               PartitionDesc pd = RelationGetPartitionDesc(rel, true);
-
-               for (int i = 0; i < pd->nparts; i++)
-               {
-                       Relation        childrel = table_open(pd->oids[i], 
AccessShareLock);
-
-                       if (RelationGetForm(childrel)->relkind == 
RELKIND_FOREIGN_TABLE)
-                               has_foreign_tbl = true;
-                       else
-                               has_foreign_tbl = 
check_partrel_has_foreign_table(childrel);
-
-                       table_close(childrel, NoLock);
-
-                       if (has_foreign_tbl)
-                               break;
-               }
-       }
-
-       return has_foreign_tbl;
-}
-
-/*
- * 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.
- *
- * 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.
- *
- *
- * check_foreign_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.
- *
- * 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.
- */
-void
-check_foreign_tables(Oid schemaid, char *pubname)
-{
-       Relation        classRel;
-       Relation        partRel;
-       ScanKeyData key[3];
-       int                     keycount = 0;
-       TableScanDesc scan;
-       HeapTuple       tuple;
-
-       classRel = table_open(RelationRelationId, AccessShareLock);
-       partRel = table_open(PartitionedRelationId, ShareLock);
-
-       /* Get the root nodes of partitioned table */
-       ScanKeyInit(&key[keycount++],
-                               Anum_pg_class_relkind,
-                               BTEqualStrategyNumber, F_CHAREQ,
-                               CharGetDatum(RELKIND_PARTITIONED_TABLE));
-
-       ScanKeyInit(&key[keycount++],
-                               Anum_pg_class_relispartition,
-                               BTEqualStrategyNumber, F_BOOLEQ,
-                               BoolGetDatum(false));
-
-       /* If schema id is provided check partitioned table in that schema */
-       if (OidIsValid(schemaid))
-               ScanKeyInit(&key[keycount++],
-                                       Anum_pg_class_relnamespace,
-                                       BTEqualStrategyNumber, F_OIDEQ,
-                                       schemaid);
-
-       scan = table_beginscan_catalog(classRel, keycount, key);
-       while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
-       {
-               Form_pg_class relForm = (Form_pg_class) GETSTRUCT(tuple);
-               Relation        pubrel = table_open(relForm->oid, 
AccessShareLock);
-
-               if (check_partrel_has_foreign_table(pubrel))
-                       ereport(ERROR,
-                                       
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                                        errmsg("cannot set parameter \"%s\" to 
true for publication \"%s\"",
-                                                       
"publish_via_partition_root", pubname),
-                                        errdetail("partition table \"%s\" in 
publication contains a foreign partition",
-                                                          
get_rel_name(relForm->oid))));
-
-               table_close(pubrel, NoLock);
-       }
-
-       table_endscan(scan);
-       table_close(classRel, AccessShareLock);
-       table_close(partRel, NoLock);
-}
diff --git a/src/backend/commands/foreigncmds.c 
b/src/backend/commands/foreigncmds.c
index 98c1c82db61..c6843a9c30a 100644
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -1424,55 +1424,6 @@ CreateForeignTable(CreateForeignTableStmt *stmt, Oid 
relid)
 
        ftrel = table_open(ForeignTableRelationId, RowExclusiveLock);
 
-       /*
-        * Check if it is a foreign partition and the partitioned table is not
-        * published or published with publish_via_partition_root option as 
false.
-        * See check_foreign_tables for details.
-        */
-       if (stmt->base.partbound != NULL)
-       {
-               RangeVar   *root = linitial_node(RangeVar, 
stmt->base.inhRelations);
-               Relation        rootrel = table_openrv(root, AccessShareLock);
-
-               if (RelationGetForm(rootrel)->relkind == 
RELKIND_PARTITIONED_TABLE)
-               {
-                       Oid                     schemaid = 
RelationGetNamespace(rootrel);
-                       List       *puboids = 
GetRelationPublications(rootrel->rd_id);
-                       List       *ancestors;
-
-                       puboids = list_concat_unique_oid(puboids, 
GetAllTablesPublications());
-                       puboids = list_concat_unique_oid(puboids, 
GetSchemaPublications(schemaid));
-                       ancestors = get_partition_ancestors(rootrel->rd_id);
-
-                       foreach_oid(ancestor, ancestors)
-                       {
-                               puboids = list_concat_unique_oid(puboids,
-                                                                               
                 GetRelationPublications(ancestor));
-                               schemaid = get_rel_namespace(ancestor);
-                               puboids = list_concat_unique_oid(puboids,
-                                                                               
                 GetSchemaPublications(schemaid));
-                       }
-                       list_free(ancestors);
-
-                       foreach_oid(puboid, puboids)
-                       {
-                               Publication *pub = GetPublication(puboid);
-
-                               if (pub->pubviaroot)
-                                       ereport(ERROR,
-                                                       
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-                                                        errmsg("cannot create 
table foreign partition \"%s\"",
-                                                                       
get_rel_name(relid)),
-                                                        errdetail("partition 
table \"%s\" is published with option publish_via_partition_root",
-                                                                          
RelationGetRelationName(rootrel))));
-                       }
-
-                       list_free(puboids);
-               }
-
-               table_close(rootrel, AccessShareLock);
-       }
-
        /*
         * For now the owner cannot be specified on create. Use effective user 
ID.
         */
diff --git a/src/backend/commands/publicationcmds.c 
b/src/backend/commands/publicationcmds.c
index b06960e8e22..54f2b505e3b 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -35,6 +35,7 @@
 #include "commands/publicationcmds.h"
 #include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
+#include "partitioning/partdesc.h"
 #include "parser/parse_clause.h"
 #include "parser/parse_collate.h"
 #include "parser/parse_relation.h"
@@ -917,7 +918,7 @@ CreatePublication(ParseState *pstate, CreatePublicationStmt 
*stmt)
        {
                /* Check if any partitioned table has foreign partition */
                if (publish_via_partition_root)
-                       check_foreign_tables(InvalidOid, stmt->pubname);
+                       publication_check_foreign_parts(InvalidOid, 
stmt->pubname);
 
                /* Invalidate relcache so that publication info is rebuilt. */
                CacheInvalidateRelcacheAll();
@@ -1086,45 +1087,42 @@ AlterPublicationOptions(ParseState *pstate, 
AlterPublicationStmt *stmt,
 
        /*
         * If publish_via_partition_root is set to true, check if the 
publication
-        * already have any foreign partition. See check_foreign_tables for 
details.
+        * has any foreign partition.  See publication_check_foreign_parts for
+        * details.
         */
        if (publish_via_partition_root_given && publish_via_partition_root)
        {
-               List       *schemaoids = NIL;
-               List       *relids = NIL;
-
                char       *pubname = stmt->pubname;
+               List       *schemaoids;
+               List       *relids;
 
                if (pubform->puballtables)
-                       check_foreign_tables(InvalidOid, pubname);
+                       publication_check_foreign_parts(InvalidOid, pubname);
 
                schemaoids = GetPublicationSchemas(pubform->oid);
-
                foreach_oid(schemaoid, schemaoids)
-                       check_foreign_tables(schemaoid, pubname);
-
-               list_free(schemaoids);
+                       publication_check_foreign_parts(schemaoid, pubname);
 
                relids = GetPublicationRelations(pubform->oid, 
PUBLICATION_PART_ROOT);
-
                foreach_oid(relid, relids)
                {
-                       Relation        pubrel = table_open(relid, 
AccessShareLock);
+                       Relation        pubrel;
 
-                       if (check_partrel_has_foreign_table(pubrel))
-                       {
+                       if (get_rel_relkind(relid) != RELKIND_PARTITIONED_TABLE)
+                               continue;
+
+                       pubrel = table_open(relid, AccessShareLock);
+
+                       if (RelationHasForeignPartition(pubrel))
                                ereport(ERROR,
-                                               
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                                                errmsg("cannot set parameter 
\"%s\" to true for publication \"%s\"",
-                                                               
"publish_via_partition_root", pubname),
-                                                errdetail("partition table 
\"%s\" in publication contains a foreign partition",
-                                                                  
get_rel_name(relid))));
-                       }
+                                               
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                               errmsg("cannot set parameter 
\"%s\" to \"%s\" for publication \"%s\"",
+                                                          
"publish_via_partition_root", "true", pubname),
+                                               errdetail("Published 
partitioned table \"%s\" contains a partition that is a foreign table.",
+                                                                 
RelationGetRelationName(pubrel)));
 
                        table_close(pubrel, NoLock);
                }
-
-               list_free(relids);
        }
 
        /* Everything ok, form a new tuple. */
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index eed4487e2d9..b594b01e774 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1129,6 +1129,53 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid 
ownerId,
                                         errmsg("\"%s\" is not partitioned",
                                                        
RelationGetRelationName(parent))));
 
+               /*
+                * If we're creating a partition that's a foreign table, verify 
that
+                * the parent table is not in a publication with
+                * publish_via_partition_root enabled.  For details, see
+                * publication_check_foreign_parts.
+                */
+               if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
+               {
+                       Oid                     schemaid;
+                       List       *puboids;
+                       List       *ancestors;
+
+                       /* Start with publications of all tables */
+                       puboids = GetAllTablesPublications();
+
+                       /* capture all publications that include this relation 
directly */
+                       puboids = GetRelationPublications(parent->rd_id);
+                       schemaid = RelationGetNamespace(parent);
+                       puboids = list_concat(puboids, 
GetSchemaPublications(schemaid));
+
+                       /* and do the same for its ancestors, if any */
+                       ancestors = get_partition_ancestors(parent->rd_id);
+                       foreach_oid(ancestor, ancestors)
+                       {
+                               puboids = list_concat(puboids, 
GetRelationPublications(ancestor));
+                               schemaid = get_rel_namespace(ancestor);
+                               puboids = list_concat(puboids, 
GetSchemaPublications(schemaid));
+                       }
+
+                       /* Check the publish_via_partition_root bit for each of 
those */
+                       list_sort(puboids, list_oid_cmp);
+                       list_deduplicate_oid(puboids);
+                       foreach_oid(puboid, puboids)
+                       {
+                               Publication *pub = GetPublication(puboid);
+
+                               if (pub->pubviaroot)
+                                       ereport(ERROR,
+                                                       
errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                                                       errmsg("cannot create 
foreign table \"%s\" as a partition of \"%s\"",
+                                                                  
RelationGetRelationName(rel), RelationGetRelationName(parent)),
+                                                       errdetail("Partitioned 
table \"%s\" is published with option \"%s\" in publication \"%s\".",
+                                                                         
RelationGetRelationName(parent),
+                                                                         
"publish_via_partition_root", pub->name));
+                       }
+               }
+
                /*
                 * The partition constraint of the default partition depends on 
the
                 * partition bounds of every other partition. It is possible 
that
@@ -20093,58 +20140,68 @@ ATExecAttachPartition(List **wqueue, Relation rel, 
PartitionCmd *cmd,
                                 errmsg("cannot attach temporary relation of 
another session as partition")));
 
        /*
-        * Check if attachrel is a foreign table or a partitioned table with
-        * foreign partition and rel is not part of publication with option
-        * publish_via_partition_root as true.
+        * If the relation to attach is a foreign table, or a partitioned table
+        * that contains a foreign table as partition, then verify that the
+        * parent table is not in a publication with publish_via_partition_root
+        * enabled.  See publication_check_foreign_parts.
         */
        if (attachrel->rd_rel->relkind == RELKIND_FOREIGN_TABLE ||
-               check_partrel_has_foreign_table(attachrel))
+               (attachrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
+                RelationHasForeignPartition(attachrel)))
        {
-               Oid                     schemaid = RelationGetNamespace(rel);
-               List       *puboids = GetRelationPublications(rel->rd_id);
+               Oid                     schemaid;
+               List       *puboids;
                List       *ancestors;
-               char       *relname = get_rel_name(rel->rd_id);
-               char       *attachrelname = get_rel_name(attachrel->rd_id);
 
-               puboids = list_concat_unique_oid(puboids, 
GetAllTablesPublications());
-               puboids = list_concat_unique_oid(puboids, 
GetSchemaPublications(schemaid));
+               /* Start with publications of all tables */
+               puboids = GetAllTablesPublications();
+
+               /* capture all publications that include this relation directly 
*/
+               puboids = list_concat(puboids, 
GetRelationPublications(rel->rd_id));
+               schemaid = RelationGetNamespace(rel);
+               puboids = list_concat(puboids, GetSchemaPublications(schemaid));
+
+               /* and do the same for its ancestors, if any */
                ancestors = get_partition_ancestors(rel->rd_id);
-
                foreach_oid(ancestor, ancestors)
                {
-                       puboids = list_concat_unique_oid(puboids,
-                                                                               
         GetRelationPublications(ancestor));
+                       puboids = list_concat(puboids, 
GetRelationPublications(ancestor));
                        schemaid = get_rel_namespace(ancestor);
-                       puboids = list_concat_unique_oid(puboids,
-                                                                               
         GetSchemaPublications(schemaid));
+                       puboids = list_concat(puboids, 
GetSchemaPublications(schemaid));
                }
 
-               list_free(ancestors);
-
+               /* Now check the publish_via_partition_root bit for each of 
those */
+               list_sort(puboids, list_oid_cmp);
+               list_deduplicate_oid(puboids);
                foreach_oid(puboid, puboids)
                {
-                       Publication *pub = GetPublication(puboid);
+                       Publication *pub;
 
+                       pub = GetPublication(puboid);
                        if (pub->pubviaroot)
                        {
                                if (attachrel->rd_rel->relkind == 
RELKIND_FOREIGN_TABLE)
                                        ereport(ERROR,
                                                        
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
                                                         errmsg("cannot attach 
foreign table \"%s\" to partition table \"%s\"",
-                                                                       
attachrelname, relname),
-                                                        errdetail("partition 
table \"%s\" is published with option publish_via_partition_root",
-                                                                          
relname)));
+                                                                       
RelationGetRelationName(attachrel),
+                                                                       
RelationGetRelationName(rel)),
+                                                        errdetail("Partitioned 
table \"%s\" is published with option \"%s\" in publication \"%s\".",
+                                                                          
RelationGetRelationName(rel),
+                                                                          
"publish_via_partition_root",
+                                                                          
pub->name)));
                                else
                                        ereport(ERROR,
                                                        
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
                                                         errmsg("cannot attach 
table \"%s\" with foreign partition to partition table \"%s\"",
-                                                                       
attachrelname, relname),
-                                                        errdetail("partition 
table \"%s\" is published with option publish_via_partition_root",
-                                                                          
relname)));
+                                                                       
RelationGetRelationName(attachrel),
+                                                                       
RelationGetRelationName(rel)),
+                                                        errdetail("Partitioned 
table \"%s\" is published with option \"%s\" in publication \"%s\".",
+                                                                          
RelationGetRelationName(rel),
+                                                                          
"publish_via_partition_root",
+                                                                          
pub->name)));
                        }
                }
-
-               list_free(puboids);
        }
 
        /*
diff --git a/src/backend/partitioning/partdesc.c 
b/src/backend/partitioning/partdesc.c
index 328b4d450e4..b53139bafdd 100644
--- a/src/backend/partitioning/partdesc.c
+++ b/src/backend/partitioning/partdesc.c
@@ -506,3 +506,35 @@ get_default_oid_from_partdesc(PartitionDesc partdesc)
 
        return InvalidOid;
 }
+
+/*
+ * Return true if the given partitioned table ultimately contains a
+ * partition that is a foreign table, false otherwise.
+ */
+bool
+RelationHasForeignPartition(Relation rel)
+{
+       PartitionDesc pd = RelationGetPartitionDesc(rel, true);
+
+       for (int i = 0; i < pd->nparts; i++)
+       {
+               if (pd->is_leaf[i])
+               {
+                       if (get_rel_relkind(pd->oids[i]) == 
RELKIND_FOREIGN_TABLE)
+                               return true;
+               }
+               else
+               {
+                       Relation        part;
+                       bool            ret;
+
+                       part = table_open(pd->oids[i], AccessShareLock);
+                       ret = RelationHasForeignPartition(part);
+                       table_close(part, NoLock);
+                       if (ret)
+                               return true;
+               }
+       }
+
+       return false;
+}
diff --git a/src/include/catalog/pg_publication.h 
b/src/include/catalog/pg_publication.h
index e2919da2541..71ad5a6f846 100644
--- a/src/include/catalog/pg_publication.h
+++ b/src/include/catalog/pg_publication.h
@@ -179,6 +179,8 @@ extern Oid  GetTopMostAncestorInPublication(Oid puboid, 
List *ancestors,
 
 extern bool is_publishable_relation(Relation rel);
 extern bool is_schema_publication(Oid pubid);
+extern void publication_check_foreign_parts(Oid schemaid, char *pubname);
+
 extern bool check_and_fetch_column_list(Publication *pub, Oid relid,
                                                                                
MemoryContext mcxt, Bitmapset **cols);
 extern ObjectAddress publication_add_relation(Oid pubid, PublicationRelInfo 
*pri,
@@ -192,8 +194,4 @@ extern Bitmapset *pub_collist_to_bitmapset(Bitmapset 
*columns, Datum pubcols,
 extern Bitmapset *pub_form_cols_map(Relation relation,
                                                                        
PublishGencolsType include_gencols_type);
 
-extern bool check_partrel_has_foreign_table(Relation rel);
-
-extern void check_foreign_tables(Oid schemaid, char *pubname);
-
 #endif                                                 /* PG_PUBLICATION_H */
diff --git a/src/include/partitioning/partdesc.h 
b/src/include/partitioning/partdesc.h
index 34533f7004c..5fbafdc06f9 100644
--- a/src/include/partitioning/partdesc.h
+++ b/src/include/partitioning/partdesc.h
@@ -71,5 +71,6 @@ extern PartitionDesc 
PartitionDirectoryLookup(PartitionDirectory, Relation);
 extern void DestroyPartitionDirectory(PartitionDirectory pdir);
 
 extern Oid     get_default_oid_from_partdesc(PartitionDesc partdesc);
+extern bool RelationHasForeignPartition(Relation rel);
 
 #endif                                                 /* PARTDESC_H */
diff --git a/src/test/regress/expected/publication.out 
b/src/test/regress/expected/publication.out
index 6a0c245bd60..4395563fcd5 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -1939,32 +1939,32 @@ ALTER TABLE sch3.tmain ATTACH PARTITION sch3.part2 FOR 
VALUES FROM (5) TO (10);
 -- Can't create publications with publish_via_partition_root = true, if table
 -- has a foreign partition
 CREATE PUBLICATION pub1 FOR TABLE sch3.tmain WITH (publish_via_partition_root);
-ERROR:  cannot set parameter "publish_via_partition_root" to true for 
publication "pub1"
-DETAIL:  partition table "tmain" in publication contains a foreign partition
+ERROR:  cannot add partitioned table "tmain" to publication "pub1", which has 
"publish_via_partition_root" set to "true"
+DETAIL:  The table contains a partition that's a foreign table.
 CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA sch3 WITH 
(publish_via_partition_root);
-ERROR:  cannot set parameter "publish_via_partition_root" to true for 
publication "pub1"
-DETAIL:  partition table "tmain" in publication contains a foreign partition
+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 PUBLICATION pub1 FOR ALL TABLES WITH (publish_via_partition_root);
-ERROR:  cannot set parameter "publish_via_partition_root" to true for 
publication "pub1"
-DETAIL:  partition table "tmain" in publication contains a foreign partition
+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.
 -- Test when a partitioned table with foreign table as a partition is attached
 -- to partitioned table which is already published
 ALTER TABLE sch3.tmain DETACH PARTITION sch3.part2;
 CREATE PUBLICATION pub1 FOR TABLE sch3.tmain WITH (publish_via_partition_root);
 ALTER TABLE sch3.tmain ATTACH PARTITION sch3.part2 FOR VALUES FROM (5) TO (10);
 ERROR:  cannot attach table "part2" with foreign partition to partition table 
"tmain"
-DETAIL:  partition table "tmain" is published with option 
publish_via_partition_root
+DETAIL:  Partitioned table "tmain" is published with option 
"publish_via_partition_root" in publication "pub1".
 -- Can't create foreign partition of published table with
 -- publish_via_partition_root = true
 CREATE FOREIGN TABLE sch3.part3_1 PARTITION OF sch3.tmain FOR VALUES FROM (10) 
TO (15) SERVER fdw_server;
-ERROR:  cannot create table foreign partition "part3_1"
-DETAIL:  partition table "tmain" is published with option 
publish_via_partition_root
+ERROR:  cannot create foreign table "part3_1" as a partition of "tmain"
+DETAIL:  Partitioned table "tmain" is published with option 
"publish_via_partition_root" in publication "pub1".
 -- Can't attach foreign partition to published table with
 -- publish_via_partition_root = true
 CREATE FOREIGN TABLE sch3.part3_2(id int) SERVER fdw_server;
 ALTER TABLE sch3.tmain ATTACH PARTITION sch3.part3_2 FOR VALUES FROM (15) TO 
(20);
 ERROR:  cannot attach foreign table "part3_2" to partition table "tmain"
-DETAIL:  partition table "tmain" is published with option 
publish_via_partition_root
+DETAIL:  Partitioned table "tmain" is published with option 
"publish_via_partition_root" in publication "pub1".
 CREATE SCHEMA sch4;
 CREATE TABLE sch4.tmain(id int) PARTITION BY RANGE(id);
 -- publication created with FOR TABLES IN SCHEMA
@@ -1973,13 +1973,13 @@ CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA sch4 WITH 
(publish_via_partition_ro
 -- Can't create foreign partition of published table with
 -- publish_via_partition_root = true
 CREATE FOREIGN TABLE sch3.part3_1 PARTITION OF sch4.tmain FOR VALUES FROM (10) 
TO (15) SERVER fdw_server;
-ERROR:  cannot create table foreign partition "part3_1"
-DETAIL:  partition table "tmain" is published with option 
publish_via_partition_root
+ERROR:  cannot create foreign table "part3_1" as a partition of "tmain"
+DETAIL:  Partitioned table "tmain" is published with option 
"publish_via_partition_root" in publication "pub1".
 -- Can't attach foreign partition to published table with
 -- publish_via_partition_root = true
 ALTER TABLE sch4.tmain ATTACH PARTITION sch3.part3_2 FOR VALUES FROM (15) TO 
(20);
 ERROR:  cannot attach foreign table "part3_2" to partition table "tmain"
-DETAIL:  partition table "tmain" is published with option 
publish_via_partition_root
+DETAIL:  Partitioned table "tmain" is published with option 
"publish_via_partition_root" in publication "pub1".
 DROP PUBLICATION pub1;
 -- Test with publish_via_partition_root = false
 -- Foreign partitions are skipped by default
@@ -2004,8 +2004,8 @@ SELECT pubname, tablename FROM pg_publication_tables 
WHERE schemaname in ('sch3'
 -- Can't alter publish_via_partition_root to true, if publication already have
 -- foreign partition
 ALTER PUBLICATION pub1 SET (publish_via_partition_root);
-ERROR:  cannot set parameter "publish_via_partition_root" to true for 
publication "pub1"
-DETAIL:  partition table "tmain" in publication contains a foreign partition
+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.
 DROP PUBLICATION pub1;
 DROP PUBLICATION pub2;
 DROP PUBLICATION pub3;
-- 
2.39.5

Reply via email to