'On Mon, May 30, 2022 at 1:51 PM Peter Smith <smithpb2...@gmail.com> wrote: > > Here are some minor review comments for v7-0001. > > ====== > > 1. General > > Probably the commit message and all the PG docs and code comments > should be changed to refer to "publication parameters" instead of > (currently) "publication options". This is because these things are > really called "publication_parameters" in the PG docs [1]. > > All the following review comments are just examples of this suggestion.
Modified > ~~~ > > 2. Commit message > > "includes resetting the publication options," -> "includes resetting > the publication parameters," Modified > ~~~ > > 3. doc/src/sgml/ref/alter_publication.sgml > > + <para> > + The <literal>RESET</literal> clause will reset the publication to the > + default state which includes resetting the publication options, setting > + <literal>ALL TABLES</literal> flag to <literal>false</literal> and > + dropping all relations and schemas that are associated with the > publication. > </para> > > > "resetting the publication options," -> "resetting the publication > parameters," Modified > ~~~ > > 4. src/backend/commands/publicationcmds.c > > @@ -53,6 +53,14 @@ > #include "utils/syscache.h" > #include "utils/varlena.h" > > +/* CREATE PUBLICATION default values for flags and options */ > +#define PUB_DEFAULT_ACTION_INSERT true > +#define PUB_DEFAULT_ACTION_UPDATE true > +#define PUB_DEFAULT_ACTION_DELETE true > +#define PUB_DEFAULT_ACTION_TRUNCATE true > +#define PUB_DEFAULT_VIA_ROOT false > +#define PUB_DEFAULT_ALL_TABLES false > > "flags and options" -> "flags and publication parameters" Modified > ~~~ > > 5. src/backend/commands/publicationcmds.c > > +/* > + * Reset the publication. > + * > + * Reset the publication options, setting ALL TABLES flag to false and drop > + * all relations and schemas that are associated with the publication. > + */ > +static void > +AlterPublicationReset(ParseState *pstate, AlterPublicationStmt *stmt, > + Relation rel, HeapTuple tup) > > "Reset the publication options," -> "Reset the publication parameters," Modified > ~~~ > > 6. src/test/regress/sql/publication.sql > > +-- Verify that publish options and publish_via_partition_root option are > reset > +\dRp+ testpub_reset > +ALTER PUBLICATION testpub_reset RESET; > +\dRp+ testpub_reset > > SUGGESTION > -- Verify that 'publish' and 'publish_via_partition_root' publication > parameters are reset Modified, I have split this into two tests as it will help the 0002 patch to add few tests with the existing steps for 'publish' and 'publish_via_partition_root' publication parameter. Thanks for the comments. the v8 patch attached at [1] has the fixes for the same. [1] - https://www.postgresql.org/message-id/CALDaNm0sAU4s1KTLOEWv%3DrYo5dQK6uFTJn_0FKj3XG1Nv4D-qw%40mail.gmail.com Regards, Vignesh
