On Fri, May 14, 2021 at 11:02 PM vignesh C <vignes...@gmail.com> wrote: > > On Fri, May 14, 2021 at 8:56 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > > > Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> writes: > > > On Fri, May 14, 2021 at 7:58 PM vignesh C <vignes...@gmail.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
v5-0001-Fixes-in-ALTER-SUBSCRIPTION-DROP-PUBLICATION-code.patch
Description: Binary data
