Re: alter subscription drop publication fixes

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Japin Li <japinli(at)hotmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: alter subscription drop publication fixes
Date: 2021-05-15 09:28:10
Message-ID: CALj2ACUdUvGezvROqaxBsx+vvf42vvAizxC8FFwup65P=uOx6A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, May 14, 2021 at 11:02 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Fri, May 14, 2021 at 8:56 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> writes:
> > > On Fri, May 14, 2021 at 7:58 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > >> I have changed it to:
> > >> <literal>ADD</literal> adds additional publications,
> > >> - <literal>DROP</literal> removes publications from the list of
> > >> + <literal>DROP</literal> removes publications to/from the list of
> >
> > > How about "Publications are added to or dropped from the existing list
> > > of publications by <literal>ADD</literal> or <literal>DROP</literal>
> > > respectively." ?
> >
> > We generally prefer to use the active voice, so I don't think
> > restructuring the sentence that way is an improvement. The quoted
> > bit would be better left alone entirely. Or maybe split it into
> > two sentences, but keep the active voice.
>
> I felt changing it to the below was better:
> SET replaces the entire list of publications with a new list, ADD adds
> additional publications to the list of publications and DROP removes
> the publications from the list of publications.
>
> Attached patch has the change for the same.
> Thoughts?

Thanks Vignesh, the patch looks good to me and it works as expected
i.e. doesn't show up the copy_data option in the tab complete for the
alter subscription drop publication command. While on this, I observed
that the new function merge_publications and the error message crossed
the 80char limit, I adjusted that and added a commit message. Please
have a look, if that is okay, add an entry to the commit fest and pass
it on to the committer as I have no further comments.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com

Attachment Content-Type Size
v5-0001-Fixes-in-ALTER-SUBSCRIPTION-DROP-PUBLICATION-code.patch application/octet-stream 9.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2021-05-15 09:33:45 Re: Added missing tab completion for alter subscription set option
Previous Message Nitin Jadhav 2021-05-15 09:10:45 Removed extra memory allocations from create_list_bounds