On Fri, 17 Jan 2025 at 21:30, vignesh C <vignes...@gmail.com> wrote:
>
> On Fri, 17 Jan 2025 at 14:00, Sergey Tatarintsev
> <s.tatarint...@postgrespro.ru> wrote:
> >
> > Hi, hackers!
> >
> > I am looking at subscription creation command:
> >
> > CREATE SUBSCRIPTION sub CONNECTION '...' PUBLICATION pub WITH (origin =
> > none, copy_data = on);
> >
> > For now we log a warning if the publisher has subscribed to the same
> > table from some other publisher.
> > However, in case of publication with publish_via_partition_root option,
> > we will not raise such warinigs
> > because SQL command in check_publications_origin() checks only directly
> > published tables.
>
> Yes, I agree that we are checking only the directly published tables
> which is why there is no warning in this case. I'm working on a fix to
> change the check_publications_origin to check accordingly.
Attached patch has the fix for this issue which includes the partition
tables also for the publication now and throws a warning
appropriately.
Regards,
Vignesh
From 514656829ac637d494cd431518986cbea0fe6bc3 Mon Sep 17 00:00:00 2001
From: Vignesh <vignes...@gmail.com>
Date: Sat, 18 Jan 2025 10:19:12 +0530
Subject: [PATCH] Fix origin warning not thrown for publications on partition
tables
When checking if a publisher had subscribed to the same table from a different
publisher, the check only considered tables directly specified for the
publication. It did not account for cases where the publication was present on
partition tables as well. This has been fixed by including all partition tables
associated with the publication in the check.
---
src/backend/catalog/pg_publication.c | 39 +++++++++++++++++++++----
src/backend/commands/subscriptioncmds.c | 2 +-
src/include/catalog/pg_proc.dat | 9 ++++++
3 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index b89098f5e9..bfdd0c7cc5 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -1089,12 +1089,16 @@ GetPublicationByName(const char *pubname, bool missing_ok)
}
/*
- * Get information of the tables in the given publication array.
+ * Helper function for SQL callables: pg_get_publication_tables and
+ * pg_get_publication_tables_with_partitions.
*
- * Returns pubid, relid, column list, row filter for each table.
+ * If allparttables is true, retrieves tables including all the partitions
+ * for the publication.
+ * If allparttables is false, retrieves tables based on the publication's
+ * pubviaroot option.
*/
-Datum
-pg_get_publication_tables(PG_FUNCTION_ARGS)
+static Datum
+pg_get_publication_tables_internal(FunctionCallInfo fcinfo, bool allparttables)
{
#define NUM_PUBLICATION_TABLES_ELEM 4
FuncCallContext *funcctx;
@@ -1147,10 +1151,12 @@ pg_get_publication_tables(PG_FUNCTION_ARGS)
*schemarelids;
relids = GetPublicationRelations(pub_elem->oid,
+ allparttables ? PUBLICATION_PART_ALL :
pub_elem->pubviaroot ?
PUBLICATION_PART_ROOT :
PUBLICATION_PART_LEAF);
schemarelids = GetAllSchemaPublicationRelations(pub_elem->oid,
+ allparttables ? PUBLICATION_PART_ALL :
pub_elem->pubviaroot ?
PUBLICATION_PART_ROOT :
PUBLICATION_PART_LEAF);
@@ -1187,7 +1193,7 @@ pg_get_publication_tables(PG_FUNCTION_ARGS)
* data of the child table to be double-published on the subscriber
* side.
*/
- if (viaroot)
+ if (!allparttables && viaroot)
filter_partitions(table_infos);
/* Construct a tuple descriptor for the result rows. */
@@ -1298,3 +1304,26 @@ pg_get_publication_tables(PG_FUNCTION_ARGS)
SRF_RETURN_DONE(funcctx);
}
+
+/*
+ * Get information of the tables in the given publication array.
+ *
+ * Returns pubid, relid, column list, row filter for each table.
+ */
+Datum
+pg_get_publication_tables(PG_FUNCTION_ARGS)
+{
+ return pg_get_publication_tables_internal(fcinfo, false);
+}
+
+/*
+ * Get information of the tables (including all the all partitions) in the
+ * given publication array.
+ *
+ * Returns pubid, relid, column list, row filter for each table.
+ */
+Datum
+pg_get_publication_tables_with_partitions(PG_FUNCTION_ARGS)
+{
+ return pg_get_publication_tables_internal(fcinfo, true);
+}
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 2d8a71ca1e..403b4fc918 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -2116,7 +2116,7 @@ check_publications_origin(WalReceiverConn *wrconn, List *publications,
appendStringInfoString(&cmd,
"SELECT DISTINCT P.pubname AS pubname\n"
"FROM pg_publication P,\n"
- " LATERAL pg_get_publication_tables(P.pubname) GPT\n"
+ " LATERAL pg_get_publication_tables_with_partitions(P.pubname) GPT\n"
" JOIN pg_subscription_rel PS ON (GPT.relid = PS.srrelid),\n"
" pg_class C JOIN pg_namespace N ON (N.oid = C.relnamespace)\n"
"WHERE C.oid = GPT.relid AND P.pubname IN (");
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 18560755d2..349c6330b2 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -12128,6 +12128,15 @@
proargmodes => '{v,o,o,o,o}',
proargnames => '{pubname,pubid,relid,attrs,qual}',
prosrc => 'pg_get_publication_tables' },
+{ oid => '8051',
+ descr => 'get information of the tables(including all partitions) that are part of the specified publications',
+ proname => 'pg_get_publication_tables_with_partitions', prorows => '1000',
+ provariadic => 'text', proretset => 't', provolatile => 's',
+ prorettype => 'record', proargtypes => '_text',
+ proallargtypes => '{_text,oid,oid,int2vector,pg_node_tree}',
+ proargmodes => '{v,o,o,o,o}',
+ proargnames => '{pubname,pubid,relid,attrs,qual}',
+ prosrc => 'pg_get_publication_tables_with_partitions' },
{ oid => '6121',
descr => 'returns whether a relation can be part of a publication',
proname => 'pg_relation_is_publishable', provolatile => 's',
--
2.43.0
