From: | vignesh C <vignesh21(at)gmail(dot)com> |
---|---|
To: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Euler Taveira <euler(at)eulerto(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org> |
Subject: | Re: Logical Replication of sequences |
Date: | 2025-10-07 11:26:12 |
Message-ID: | CALDaNm1iCU3ban4YOKax9aCsjoCBOEdNe=47vLwbiak9AwbnTw@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, 7 Oct 2025 at 10:53, shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Mon, Oct 6, 2025 at 4:33 PM vignesh C <vignesh21(at)gmail(dot)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.
Regards,
Vignesh
From | Date | Subject | |
---|---|---|---|
Next Message | Oliver Ford | 2025-10-07 11:38:48 | Re: Questionable result from lead(0) IGNORE NULLS |
Previous Message | Dilip Kumar | 2025-10-07 11:25:49 | Re: Logical Replication of sequences |