On Thu, 1 Feb 2024 at 14:50, vignesh C <vignes...@gmail.com> wrote: > > On Wed, 31 Jan 2024 at 11:42, Hayato Kuroda (Fujitsu) > <kuroda.hay...@fujitsu.com> wrote: > > > > Dear Vignesh, > > > > Thanks for updating the patch! Here are my comments for v6. > > > > 01. > > ``` > > + <glossterm>Logical replication cluster</glossterm> > > + <glossdef> > > + <para> > > + A set of publisher and subscriber instance with publisher instance > > + replicating changes to the subscriber instance. > > + </para> > > + </glossdef> > > ``` > > > > Should we say 1:N relationship is allowed? > > I felt this need not be mentioned here, just wanted to give an > indication wherever this terminology is used, it means a set of > publisher and subscriber instances. Detail information should be added > in the logical replication related pages > > > 02. > > ``` > > @@ -70,6 +70,7 @@ PostgreSQL documentation > > pg_upgrade supports upgrades from 9.2.X and later to the current > > major release of <productname>PostgreSQL</productname>, including > > snapshot and beta releases. > > </para> > > + > > </refsect1> > > ``` > > > > Unnecessary blank. > > Removed it. > > > 03. > > ``` > > <para> > > - These are the steps to perform an upgrade > > - with <application>pg_upgrade</application>: > > + Below are the steps to perform an upgrade > > + with <application>pg_upgrade</application>. > > </para> > > ``` > > > > I'm not sure it should be included in this patch. > > This is not required in this patch, removed it. > > > 04. > > ``` > > + If the old primary is prior to version 17.0, then no slots on the > > primary > > + are copied to the new standby, so all the slots on the old standby > > must > > + be recreated manually. > > ``` > > > > I think that "all the slots on the old standby" must be created manually in > > any > > cases. Therefore, the preposition ", so" seems not correct. > > I felt that this change is not related to this patch. I'm removing > these changes from the patch. Let's handle rephrasing of the base code > change in a separate thread. > > > 05. > > ``` > > If the old primary is version 17.0 or later, then > > + only logical slots on the primary are copied to the new standby, but > > + other slots on the old standby are not copied, so must be recreated > > + manually. > > ``` > > > > How about replacing this paragraph to below? > > > > ``` > > All the slots on the old standby must be recreated manually. If the old > > primary > > is version 17.0 or later, then only logical slots on the primary are copied > > to the > > new standby. > > ``` > > I felt that this change is not related to this patch. I'm removing > these changes from the patch. Let's handle rephrasing of the base code > change in a separate thread.
A new thread is started for the same and a patch is attached at [1]. Let's discuss this change at the new thread. [1] - https://www.postgresql.org/message-id/CAHv8RjJHCw0jpUo9PZxjcguzGt3j2W1_NH%3DQuREoN0nYiVdVeA%40mail.gmail.com Regards, Vignesh
