Re: Logical Replication of sequences

On Mon, Jun 30, 2025 at 3:21 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
> Please find the attached v20250630 patch set addressing above comments
> and other comments in [1],[2],[3] and [4].

Thanks for the patches. I am still in process of reviewing it but
please find few comments:

1)
+ if (pset.sversion >= 180000)
+ appendPQExpBuffer(&buf,
+ ",\n puballsequences AS \"%s\"",
+ gettext_noop("All sequences"));

The server version check throughout the patch can be modified to 19000
as a new branch is created now.

2)
+ bool all_pub; /* Special publication for all tables,
+ * sequecnes */

a) Typo: sequecnes --> sequences
b) It is not clear from the comment that when will it be true? Will it
be set when either of all-tables or all-sequences is given or does it
need both?

3)
postgres=# create publication pub1 for all sequences WITH ( PUBLISH='delete');
CREATE PUBLICATION
postgres=# create publication pub2 for all tables, sequences WITH
(PUBLISH='update');
CREATE PUBLICATION

For the first command, 'WITH ( publication_parameter..' is useless.
For the second command, it is applicable only for 'all tables'.

a) I am not sure if we even allow WITH in the first command?
b) In the second command, even if we allow it, there should be some
sort of NOTICE informing that it is applicable only to 'TABLES'.

Thoughts?

Also we allowed altering publication_parameter for all-sequences publication:

postgres=# alter publication pub1 set (publish='insert,update');
ALTER PUBLICATION

c) Should this be restricted as well? Thoughts?

thanks
Shveta