Re: Include sequences in publications created by pg_createsubscriber

On Tue, 7 Jul 2026 at 11:50, Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> 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.

Modified

> ~~~
>
> 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."

Modified

> ~~~
>
> 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."

Modified

> ~~~
>
> 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."

We haven't stated anywhere that pg_createsubscriber is intended to be
used only for upgrades, so I don't think we should mention upgrades
here. I've incorporated the rest of the suggested changess

> ~~~
>
> 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>).

Modified

> ======
> 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_%'.

The verification done here was not correct, I have changed it to
verify the command from the log contents.

> ~
>
> 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.

I felt this is not required as we are not doing any ALTER PUBLICATION.

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

I don't think this needs to be mentioned in the pg_createsubscriber
documentation. The behavior of sequence synchronization, including the
need to run ALTER SUBSCRIPTION ... REFRESH SEQUENCES is more
appropriately documented with the REFRESH SEQUENCES command and the
logical replication documentation. Since we have given a link to
REFRESH SEQUENCES, it should be enough.

> ~~~
>
> 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)

Same reason as above, it need not be mentioned here.

Thanks for the comments, the attached v8 version patch has the changes
for the same. This patch also includes the changes to handle
Ashutosh's comment from [1] with appropriate rewording.

[1] - https://www.postgresql.org/message-id/CAE9k0P%3D9rReCa4CMTWfaYe3ZhE8PqrNCRBnv6QcVPLQm_%2BRFQw%40mail.gmail.com

Regards,
Vignesh