public inbox for [email protected]  
help / color / mirror / Atom feed
From: Peter Smith <[email protected]>
To: Shlok Kyal <[email protected]>
Cc: vignesh C <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Include sequences in publications created by pg_createsubscriber
Date: Tue, 7 Jul 2026 16:20:23 +1000
Message-ID: <CAHut+Puu6AvPWG=dbcbEVunPJVsaUz19tUpqJhqg0yU2W97XWQ@mail.gmail.com> (raw)
In-Reply-To: <CANhcyEV-B0FzrZThZ3XAd0x5ZTj5BFfHQQ3OrHAJSTzeEJQzSQ@mail.gmail.com>
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>

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.

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

1.
+   replica from a physical standby server.  By default,
+   <application>pg_createsubscriber</application> configures
+   <link linkend="logical-replication">logical replication</link> by
+   automatically creating internal publication and subscription objects. In
+   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 — 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 using
+   internally generated publications, they automatically include all sequences
+   in the database; however, their values are not automatically synchronized.

"they" is ambiguous — 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 like
+   the default mode, their values will require manual synchronization after the
+   upgrade.

"their values will require manual synchronization" — "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 —
consider "as with the default mode."

~~~

4.
+  <para>
+   To synchronize sequence values, run
+   <link linkend="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="sql-createpublication-params-for-all-tables">
+      <literal>FOR ALL TABLES</literal></link> and
+      <link linkend="sql-createpublication-params-for-all-sequences">
+      <literal>ALL SEQUENCES</literal></link>. Otherwise, it uses the
+      user-specified publication, which includes the objects defined by that
+      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="sql-createpublication-params-for-all-tables">
<literal>FOR ALL TABLES</literal></link> and <link
linkend="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>).

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

6.
+$result = $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 — if there are multiple publications
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.

======
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 — 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)

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






view thread (23+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Include sequences in publications created by pg_createsubscriber
  In-Reply-To: <CAHut+Puu6AvPWG=dbcbEVunPJVsaUz19tUpqJhqg0yU2W97XWQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox