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 |
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 |