On Tue, 7 Oct 2025 at 10:53, shveta malik <shveta.malik@gmail.com> wrote:
>
> 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.
Moved it to 0001 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'.
The second error message is changed now based on another comment, so
both of them will have as now.
> b) Shouldn't the error msg start with lower case (second one)?
Updated
> 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'
Updated
> 4)
> +/*
> + * Publication types supported by FOR ALL ...
> + */
> +typedef enum PublicationAllObjType
>
> Should it be:
> 'Types of objects supported by FOR ALL publications'
Modified
> 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).
Modified
> 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.
Modified
Thanks for the comments, the v20251007 patch attached at [1] has the
changes for the same.
[1] - https://www.postgresql.org/message-id/CALDaNm3NJBiXpQ3uY8%3DXhSPd6jBn2rTJS6wJZSFo6m2pzW5hqw%40mail.gmail.com
Regards,
Vignesh
