Re: Logical Replication of sequences

On Mon, Oct 6, 2025 at 4:33 PM vignesh C <vignesh21@gmail.com> wrote:
>
>
> Here is the rebased remaining patches.
>

Thank You for the patches, please find a few comment on 001:

1)
Shall we have 'pg_publication_sequences' created in the first patch
itself to help verify which all sequences are added to ALL SEQ
publications? Currently it is in 4th patch.

2)
postgres=# create publication pub1 for all sequences WITH(publish='insert');
ERROR:  publication parameters are not supported for publications
defined as FOR ALL SEQUENCES

postgres=# alter publication pub1 add table tab1;
ERROR:  Tables or sequences cannot be added to or dropped from
publication defined FOR ALL TABLES, ALL SEQUENCES, or both

a) First msg has 'as', while second does not. Shall we make both the
same? I think we can get rid of 'as'.
b) Shouldn't the error msg start with lower case (second one)?


3)
+ * Process all_objects_list to set all_tables/all_sequences.

can we please replace 'all_tables/all_sequences' with 'all_tables
and/or all_sequences'

4)
+/*
+ * Publication types supported by FOR ALL ...
+ */
+typedef enum PublicationAllObjType

Should it be:
'Types of objects supported by FOR ALL publications'

5)
+-- Specifying both ALL TABLES and ALL SEQUENCES along with WITH
clause should throw a warning
+SET client_min_messages = 'NOTICE';
+CREATE PUBLICATION regress_pub_for_allsequences_alltables_withcaluse
FOR ALL SEQUENCES, ALL TABLES WITH (publish = 'insert');
+NOTICE:  publication parameters are not applicable to sequence
synchronization and will be ignored

Comment can be changed to say it will emit/raise a NOTICE (instead of warning).

6)
commit msg:
--
Note: This patch currently supports only the "ALL SEQUENCES" clause.
Handling of clauses such as "FOR SEQUENCE" and "FOR SEQUENCES IN SCHEMA"
will be addressed in a subsequent patch.
--

This seems misleading, as we are not planning the "FOR SEQUENCE" in
the current set of patches, maybe we can rephrase it a bit.

thanks
Shveta