Re: Skipping schema changes in publication - Mailing list pgsql-hackers
From | vignesh C |
---|---|
Subject | Re: Skipping schema changes in publication |
Date | |
Msg-id | CALDaNm0iZZDB300Dez_97S8G6_RW5QpQ8ef6X3wq8tyK-8wnXQ@mail.gmail.com Whole thread Raw |
In response to | RE: Skipping schema changes in publication ("osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>) |
Responses |
RE: Skipping schema changes in publication
Re: Skipping schema changes in publication Re: Skipping schema changes in publication Re: Skipping schema changes in publication |
List | pgsql-hackers |
On Mon, May 16, 2022 at 8:32 AM osumi.takamichi@fujitsu.com <osumi.takamichi@fujitsu.com> wrote: > > On Saturday, May 14, 2022 10:33 PM vignesh C <vignesh21@gmail.com> wrote: > > Thanks for the comments, the attached v5 patch has the changes for the same. > > Also I have made the changes for SKIP Table based on the new syntax, the > > changes for the same are available in > > v5-0002-Skip-publishing-the-tables-specified-in-EXCEPT-TA.patch. > Hi, > > > Thank you for updating the patch. > I'll share few minor review comments on v5-0001. > > > (1) doc/src/sgml/ref/alter_publication.sgml > > @@ -73,12 +85,13 @@ ALTER PUBLICATION <replaceable class="parameter">name</replaceable> RENAME TO <r > Adding a table to a publication additionally requires owning that table. > The <literal>ADD ALL TABLES IN SCHEMA</literal> and > <literal>SET ALL TABLES IN SCHEMA</literal> to a publication requires the > - invoking user to be a superuser. To alter the owner, you must also be a > - direct or indirect member of the new owning role. The new owner must have > - <literal>CREATE</literal> privilege on the database. Also, the new owner > - of a <literal>FOR ALL TABLES</literal> or <literal>FOR ALL TABLES IN > - SCHEMA</literal> publication must be a superuser. However, a superuser can > - change the ownership of a publication regardless of these restrictions. > + invoking user to be a superuser. <literal>RESET</literal> of publication > + requires the invoking user to be a superuser. To alter the owner, you must > ... > > > I suggest to combine the first part of your change with one existing sentence > before your change, to make our description concise. > > FROM: > "The <literal>ADD ALL TABLES IN SCHEMA</literal> and > <literal>SET ALL TABLES IN SCHEMA</literal> to a publication requires the > invoking user to be a superuser. <literal>RESET</literal> of publication > requires the invoking user to be a superuser." > > TO: > "The <literal>ADD ALL TABLES IN SCHEMA</literal>, > <literal>SET ALL TABLES IN SCHEMA</literal> to a publication and > <literal>RESET</literal> of publication requires the invoking user to be a superuser." Modified > > (2) typo > > +++ b/src/backend/commands/publicationcmds.c > @@ -53,6 +53,13 @@ > #include "utils/syscache.h" > #include "utils/varlena.h" > > +#define PUB_ATION_INSERT_DEFAULT true > +#define PUB_ACTION_UPDATE_DEFAULT true > > > Kindly change > FROM: > "PUB_ATION_INSERT_DEFAULT" > TO: > "PUB_ACTION_INSERT_DEFAULT" Modified > > (3) src/test/regress/expected/publication.out > > +-- Verify that only superuser can reset a publication > +ALTER PUBLICATION testpub_reset OWNER TO regress_publication_user2; > +SET ROLE regress_publication_user2; > +ALTER PUBLICATION testpub_reset RESET; -- fail > > > We have "-- fail" for one case in this patch. > On the other hand, isn't better to add "-- ok" (or "-- success") for > other successful statements, > when we consider the entire tests description consistency ? We generally do not mention success comments for all the success cases as that might be an overkill. I felt it is better to keep it as it is. Thoughts? The attached v6 patch has the changes for the same. Regards, Vignesh
Attachment
pgsql-hackers by date: