| From: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
|---|---|
| To: | vignesh C <vignesh21(at)gmail(dot)com> |
| Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, shveta malik <shveta(dot)malik(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>, 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-08 09:11:27 |
| Message-ID: | CAFiTN-u8GyobnYbyS5aaSKrz8odfMEGv+SEBTvMxm4mtAeG4jg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, Oct 7, 2025 at 4:52 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Tue, 7 Oct 2025 at 12:09, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
I think the patch is mostly LGTM, I have 2 suggestions, see if you
think this is useful.
1.
postgres[1390699]=# CREATE PUBLICATION pub FOR ALL SEQUENCES, ALL
TABLES WITH (publish = insert);
NOTICE: 55000: publication parameters are not applicable to sequence
synchronization and will be ignored
LOCATION: CreatePublication, publicationcmds.c:905
IMHO this notice seems confusing, from this it appears that (publish =
insert) is ignored completely, but actually it is is not ignored for
table, should we explicitely say that it will be ignored only for
sequences. Something like below?
"publication parameters are not applicable to sequence synchronization
so it will be used only for tables and will be ignored for sequence
synchronization"
or
"publication parameters are not applicable to sequence synchronization
so it will be ignored for the sequence synchronization"
2.
+ if (schemaidlist && (pubform->puballtables || pubform->puballsequences))
+ {
+ if (pubform->puballtables && pubform->puballsequences)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("publication \"%s\" is defined as FOR ALL TABLES, ALL SEQUENCES",
+ NameStr(pubform->pubname)),
+ errdetail("Schemas cannot be added to or dropped from FOR ALL
TABLES, ALL SEQUENCES publications."));
+ else if (pubform->puballtables)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("publication \"%s\" is defined as FOR ALL TABLES",
+ NameStr(pubform->pubname)),
+ errdetail("Schemas cannot be added to or dropped from FOR ALL TABLES
publications."));
+ else
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("publication \"%s\" is defined as FOR ALL SEQUENCES",
+ NameStr(pubform->pubname)),
+ errdetail("Schemas cannot be added to or dropped from FOR ALL
SEQUENCES publications."));
+ }
Can't we make this a single generic error message instead of
duplicating for each combination? Something like
errmsg("publication \"%s\" is defined as FOR ALL TABLES or ALL SEQUENCES",
NameStr(pubform->pubname)),
errdetail("Schemas cannot be added to or dropped from FOR ALL TABLES
or ALL SEQUENCES publications."));
--
Regards,
Dilip Kumar
Google
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Vik Fearing | 2025-10-08 09:20:48 | Re: Allow ON CONFLICT DO UPDATE to return EXCLUDED values |
| Previous Message | Amit Kapila | 2025-10-08 09:10:21 | Re: Logical Replication of sequences |