MIME-Version: 1.0
References: 
 <CALDaNm2RYQRLjexRJOpiZOFkspp8PRgoh6S7d2y003TJ_TSSZA@mail.gmail.com>
 <CAHut+PtGOeXr69qQRgq_DW=BEEix_6bDsgmR8Vk4_qbNy5BvAA@mail.gmail.com>
 <CALDaNm05u0UvteGQf_9wKbUFujO0JvJk=GnTeZyxsdB0bmPtcg@mail.gmail.com>
 <CAHut+PsKCgwTP-6TxdwcNT5FT3dTyJiVfEKMPJR1K3df0EbOqQ@mail.gmail.com>
 <CALDaNm39kG3iweFqyS0=3UNOSzZn_13Cq5iEKqcyG7apXGPJbw@mail.gmail.com>
 <CAHut+PuaM2BB34207YDkdr85neU1Xe2+QkYY3aDMUinFF=96gQ@mail.gmail.com>
 <CALDaNm3Kh83ufGbgJqc8Frn=UvqC3LAd43eKS=FRuEWaRxbp4w@mail.gmail.com>
 <CANhcyEWFi-FkBERbKcK+TKVnBThF1_qUbZ2gC-Lo6P4z7_05EQ@mail.gmail.com>
 <CALDaNm0G=RXa+mXwrqSLYATdT4NvhZ+bv6yrH8pB+xWzBDTSxQ@mail.gmail.com>
 <CAHut+Pt4-P4N7m_SWVWvQzjh0hVd0JVgCqyxL5nU_eYKqMGxhA@mail.gmail.com>
 <CALDaNm0F9oX18eH5MbNRWS5uux2KneDc2RwX__+7vm7nvgHsXg@mail.gmail.com>
 <CANhcyEWKiBcV-VjNzCy3DZGmJ=_kso=yOnOVzNDLEgHBs42c0g@mail.gmail.com>
 <CANhcyEV-B0FzrZThZ3XAd0x5ZTj5BFfHQQ3OrHAJSTzeEJQzSQ@mail.gmail.com>
In-Reply-To: 
 <CANhcyEV-B0FzrZThZ3XAd0x5ZTj5BFfHQQ3OrHAJSTzeEJQzSQ@mail.gmail.com>
From: Peter Smith <smithpb2250@gmail.com>
Date: Tue, 7 Jul 2026 16:20:23 +1000
Message-ID: 
 <CAHut+Puu6AvPWG=dbcbEVunPJVsaUz19tUpqJhqg0yU2W97XWQ@mail.gmail.com>
Subject: Re: Include sequences in publications created by pg_createsubscriber
To: Shlok Kyal <shlok.kyal.oss@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>,
	PostgreSQL Hackers <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/CAHut%2BPuu6AvPWG%3DdbcbEVunPJVsaUz19tUpqJhqg0yU2W97XWQ%40mail.gmail.com>
Precedence: bulk

Hi Shlok.

Although I'd previously said "v6 LGTM", I decided to feed the v7 patch
through my AI to see what else it might come up with.

It got a bit pedantic, but OTOH the suggestions really *do* seem like
improvements to me, so I feel obliged to pass them on.

=3D=3D=3D=3D=3D=3D
doc/src/sgml/ref/pg_createsubscriber.sgml

1.
+   replica from a physical standby server.  By default,
+   <application>pg_createsubscriber</application> configures
+   <link linkend=3D"logical-replication">logical replication</link> by
+   automatically creating internal publication and subscription objects. I=
n
+   this mode, a publication and subscription are created for each database=
, and
+   the publications include all tables and sequences in their respective
+   databases.

"In this mode" is vague =E2=80=94 consider "When using internally generated
publications," to be consistent with the terminology used later in the
same paragraph.

~~~

2.
+   which ensures each table is brought up to a synchronized state. When us=
ing
+   internally generated publications, they automatically include all seque=
nces
+   in the database; however, their values are not automatically synchroniz=
ed.

"they" is ambiguous =E2=80=94 could refer to publications or sequences.
Suggest: "however, sequence values are not automatically
synchronized."

~~~

3.
+   For user-specified publications, sequences are only replicated if they =
were
+   explicitly added to those publications prior to running the tool, and l=
ike
+   the default mode, their values will require manual synchronization afte=
r the
+   upgrade.

"their values will require manual synchronization" =E2=80=94 "will require"=
 is
slightly awkward. Suggest "their values must be manually synchronized
after the upgrade."

Also "like the default mode" is a slightly informal comparison =E2=80=94
consider "as with the default mode."

~~~

4.
+  <para>
+   To synchronize sequence values, run
+   <link linkend=3D"sql-altersubscription-params-refresh-sequences">
+   <command>ALTER SUBSCRIPTION ... REFRESH SEQUENCES</command></link>.
+  </para>
```
This is quite abrupt as a standalone paragraph. It doesn't say *when*
to run it or *which* subscription to run it against. Something like:

"To synchronize sequence values after the upgrade, run `ALTER
SUBSCRIPTION ... REFRESH SEQUENCES` on each subscription at the target
server."

~~~

5.
+      specified, <application>pg_createsubscriber</application> creates a
+      publication using <link
linkend=3D"sql-createpublication-params-for-all-tables">
+      <literal>FOR ALL TABLES</literal></link> and
+      <link linkend=3D"sql-createpublication-params-for-all-sequences">
+      <literal>ALL SEQUENCES</literal></link>. Otherwise, it uses the
+      user-specified publication, which includes the objects defined by th=
at
+      publication.  If the <option>--publication</option> option is not
+      specified, the publication has the following name pattern:

SUGGESTION avoid repeating --publication by rearranging like below:

Create a publication and replication slot for each specified database
on the source server. If the <option>--publication</option> option is
specified, the user-specified publication is used, which includes the
objects defined by that publication. Otherwise,
<application>pg_createsubscriber</application> creates a publication
using <link linkend=3D"sql-createpublication-params-for-all-tables">
<literal>FOR ALL TABLES</literal></link> and <link
linkend=3D"sql-createpublication-params-for-all-sequences"> <literal>ALL
SEQUENCES</literal></link>. This generated publication has the
following name pattern:
<quote><literal>pg_createsubscriber_%u_%x</literal></quote>
(parameter:
database <parameter>oid</parameter>, random <parameter>int</parameter>).

=3D=3D=3D=3D=3D=3D
src/bin/pg_basebackup/t/040_pg_createsubscriber.pl

6.
+$result =3D $node_p->safe_psql($db2,
+ "SELECT puballtables, puballsequences FROM pg_publication");
+is($result, qq(t|t),
+ "publication is created with both all-tables and all-sequences enabled");

6a. The query has no WHERE clause =E2=80=94 if there are multiple publicati=
ons
it could return multiple rows and give a misleading result. Should
filter by publication name, e.g. WHERE pubname LIKE
'pg_createsubscriber_%'.

~

6b. Should there also be a test that verifies a user-specified
publication is not modified (i.e. does not have ALL SEQUENCES added)
when --publication is specified? That would give better coverage of
the two code paths.

=3D=3D=3D=3D=3D=3D
GENERAL

7.
The commit message says "Administrators can then run `ALTER
SUBSCRIPTION ... REFRESH SEQUENCES` after the upgrade" but the
documentation does not make this prominent enough. Given this is the
key action administrators need to take, it deserves a more prominent
note =E2=80=94 perhaps a `<note>` or `<tip>` block rather than a brief
standalone paragraph.

(I think this is related with point 4)

~~~

8.
There is no mention anywhere in the patch of what happens to sequence
values if the administrator forgets to run REFRESH SEQUENCES before
promoting the subscriber. A note about the risk (duplicate key
violations, incorrect nextval results) would make the documentation
more complete and help administrators understand why this step
matters.

(Again, I think this is related with point 4)

=3D=3D=3D=3D=3D=3D
Kind Regards,
Peter Smith.
Fujitsu Australia