MIME-Version: 1.0
References: 
 <CANhcyEVQ866NA5NbTi-aQhPj2zN_vbNb_ZW=E_P2foZYrW9VbA@mail.gmail.com>
 <202505101759.6qlloepaeddl@alvherre.pgsql>
 <CAA4eK1KavSbVdk9OeQDV5yc0FVy_rFTam5HPozqYSoM=a=8hXg@mail.gmail.com>
 <CANhcyEVUf5PEmahQvcc8GgCzxaAnJ8kTrir0Am_7O4SkZnpkQA@mail.gmail.com>
In-Reply-To: 
 <CANhcyEVUf5PEmahQvcc8GgCzxaAnJ8kTrir0Am_7O4SkZnpkQA@mail.gmail.com>
From: Ajin Cherian <itsajin@gmail.com>
Date: Wed, 4 Jun 2025 20:41:59 +1000
Message-ID: 
 <CAFPTHDYYq-Jx4RawpPNu78nq1TdPr7pZRWSdSm=eAejYr_wcag@mail.gmail.com>
Subject: Re: Restrict publishing of partitioned table with a foreign table as
 partition
To: Shlok Kyal <shlok.kyal.oss@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>,
 =?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@alvh.no-ip.org>,
	vignesh C <vignesh21@gmail.com>,
 Sergey Tatarintsev <s.tatarintsev@postgrespro.ru>,
	pgsql-hackers@lists.postgresql.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAFPTHDYYq-Jx4RawpPNu78nq1TdPr7pZRWSdSm%3DeAejYr_wcag%40mail.gmail.com>
Precedence: bulk

On Tue, May 20, 2025 at 2:33=E2=80=AFAM Shlok Kyal <shlok.kyal.oss@gmail.co=
m> wrote:
>
> This approach seems better to me. I have created a patch with the
> above approach.
>
> Thanks and Regards,
> Shlok Kyal

Some quick comments on the patch:
1. In doc/src/sgml/ref/create_subscription.sgml:
+    has partitioned table with foreign table as partition. If this scenari=
o is
+    detected we ERROR is logged to the user.
+   </para>
+

Should be: "If this scenario is detected an ERROR is logged to the
user." (remove "we").

In src/backend/commands/subscriptioncmds.c:
2. The comment header:
+ * This function is in charge of detecting if publisher with
+ * publish_via_partition_root=3Dtrue publishes a partitioned table that ha=
s a
+ * foreign table as a partition.

Add "and throw an error if found" at the end of that sentence to
correctly describe what the function does.

3.
+    appendStringInfoString(&cmd,
+                           "SELECT DISTINCT P.pubname AS pubname "
+                           "from pg_catalog.pg_publication p, LATERAL "
+                           "pg_get_publication_tables(p.pubname) gpt, LATE=
RAL "
+                           "pg_partition_tree(gpt.relid) gt JOIN
pg_catalog.pg_foreign_table ft ON "
+                           "ft.ftrelid =3D gt.relid WHERE p.pubviaroot
=3D true AND  p.pubname IN (");

use FROM rather than from to maintain SQL style consistency.

4.
+                errdetail_plural("The subscription being created on a
publication (%s) with publish_via_root_partition =3D true and contains
partitioned tables with foreign table as partition ",
+                                 "The subscription being created on
publications (%s) with publish_via_root_partition =3D true and contains
partitioned tables with foreign table as partition ",
+                                 list_length(publist), pubnames->data),

I think you meant "publish_via_partition_root" here and not
"publish_via_root_partition ".

regards,
Ajin Cherian
Fujitsu Australia