Re: Skipping schema changes in publication

public inbox for [email protected]  
help / color / mirror / Atom feed

From: vignesh C <[email protected]>
To: Peter Smith <[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: Wed, 18 May 2022 23:17:27 +0530
Message-ID: <CALDaNm3JSTRTWOvv=dGesO5Gb98kfoyZT9DSu+yP_LeUL=40zA@mail.gmail.com> (raw)
In-Reply-To: <CAHut+PtskkbsV6h=9jNF4zoeWFK0L3vWAWjmmGdbR6FYkecVzA@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>
	<CAHut+PtskkbsV6h=9jNF4zoeWFK0L3vWAWjmmGdbR6FYkecVzA@mail.gmail.com>

On Mon, May 16, 2022 at 2:53 PM Peter Smith <[email protected]> wrote:
>
> Below are my review comments for v5-0001.
>
> There is some overlap with comments recently posted by Osumi-san [1].
>
> (I also have review comments for v5-0002; will post them tomorrow)
>
> ======
>
> 1. Commit message
>
> This patch adds a new RESET clause to ALTER PUBLICATION which will reset
> the publication to default state which includes resetting the publication
> options, setting ALL TABLES option to false and dropping the relations and
> schemas that are associated with the publication.
>
> SUGGEST
> "to default state" -> "to the default state"
> "ALL TABLES option" -> "ALL TABLES flag"

Modified

> ~~~
>
> 2. 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> option to <literal>false</literal> and
> +   dropping all relations and schemas that are associated with the publication.
>    </para>
>
> "ALL TABLES option" -> "ALL TABLES flag"

Modified

> ~~~
>
> 3. doc/src/sgml/ref/alter_publication.sgml
>
> +   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
>
> SUGGESTION
> To <literal>RESET</literal> a publication requires the invoking user
> to be a superuser.

 I have combined it with the earlier sentence.

> ~~~
>
> 4. 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
> +#define PUB_ACTION_DELETE_DEFAULT true
> +#define PUB_ACTION_TRUNCATE_DEFAULT true
> +#define PUB_VIA_ROOT_DEFAULT false
> +#define PUB_ALL_TABLES_DEFAULT false
>
> 4a.
> Typo: "ATION" -> "ACTION"

Modified

> 4b.
> I think these #defines deserve a 1 line comment.
> e.g.
> /* CREATE PUBLICATION default values for flags and options */

Added comment

> 4c.
> Since the "_DEFAULT" is a common part of all the names, maybe it is
> tidier if it comes first.
> e.g.
> #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

Modified

The v6 patch attached at [1] has the changes for the same.
[1] - https://www.postgresql.org/message-id/CALDaNm0iZZDB300Dez_97S8G6_RW5QpQ8ef6X3wq8tyK-8wnXQ%40mail.gma...

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]
  Subject: Re: Skipping schema changes in publication
  In-Reply-To: <CALDaNm3JSTRTWOvv=dGesO5Gb98kfoyZT9DSu+yP_LeUL=40zA@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