public inbox for [email protected]  
help / color / mirror / Atom feed
From: vignesh C <[email protected]>
To: Peter Smith <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Skipping schema changes in publication
Date: Fri, 3 Jun 2022 15:40:55 +0530
Message-ID: <CALDaNm1VnoPFpUneukoaKmvVdyuUSSi=_pPGYTGbrSZrR2acBQ@mail.gmail.com> (raw)
In-Reply-To: <CAHut+PuHvZQL6aksQ2wJ5hE6Xw=MfJCaBU_=UMEZB-qAgSn9UA@mail.gmail.com>
References: <CALDaNm3=JrucjhiiwsYQw5-PGtBHFONa6F7hhWCXMsGvh=tamA@mail.gmail.com>
	<CALj2ACVOzhs+BD+abFV2x4oKJdsDNd6SgsE7r8UjnZDCKGEckA@mail.gmail.com>
	<CAA4eK1K6Kr88d2S0zFdHRMyuoaZeNh+ktU+oigmCuD09_x_-+g@mail.gmail.com>
	<CAHut+PsvC-NezO3MJkdyEz=G1QRje2LntjwhQiEeVbmhOQuBMA@mail.gmail.com>
	<CALDaNm18VH2j8cTqfELHQ=0ZNognbGBhbHPteJenWQC6C2dueQ@mail.gmail.com>
	<CALDaNm0k_0Ccj47wzJzzPFwgQB7w=R5+Q2_nSqYrmMmjhmcRUw@mail.gmail.com>
	<CAHut+Pv_0DwyWoGQNMF+G2AGqMuJTzWQKRtmxaC+=zLTPL-Zkw@mail.gmail.com>
	<CALDaNm2-GJt2HsYTkLqQ=ecm=R-vOBw1=aM_d2EiYbz39x_cTQ@mail.gmail.com>
	<TYCPR01MB8373C3120C2B3112001ED6F1EDCF9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
	<CALDaNm0iZZDB300Dez_97S8G6_RW5QpQ8ef6X3wq8tyK-8wnXQ@mail.gmail.com>
	<CAHut+PtiomM+iyAZHvb2dzfsPvRru266KuBe49hKy2n2h+m_zA@mail.gmail.com>
	<CALDaNm30KDnwX4Czi29fqLb8JBkuwqjbpj9ixwNXXox574NZqQ@mail.gmail.com>
	<CALDaNm1PfKRJsEzbKpyt=v4p3bw+_SzE+LFPsMhR5X+qs+0pPw@mail.gmail.com>
	<CAHut+PuHvZQL6aksQ2wJ5hE6Xw=MfJCaBU_=UMEZB-qAgSn9UA@mail.gmail.com>

'On Mon, May 30, 2022 at 1:51 PM Peter Smith <[email protected]> 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.g...

Regards,
Vignesh





view thread (377+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Skipping schema changes in publication
  In-Reply-To: <CALDaNm1VnoPFpUneukoaKmvVdyuUSSi=_pPGYTGbrSZrR2acBQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox