On Thu, Jun 12, 2025 at 4:00 AM Peter Smith <smithpb2...@gmail.com> wrote: > > On Wed, Jun 11, 2025 at 8:16 PM Ajin Cherian <itsa...@gmail.com> wrote: > > > > On Fri, Jun 6, 2025 at 5:07 PM Nisha Moond <nisha.moond...@gmail.com> wrote: > > > > > > > > > Attached v18 patch. > > > - patch-001: modified error messages as suggested above. > > > - patch-002: improved pg_dump docs as per Shveta's off-list suggestions. > > > > > > [1] > > > https://www.postgresql.org/message-id/CAA4eK1%2BB067G8mUJzKUEjc5KSkYq6z0utTaHey-qeRt%2BnZTNJg%40mail.gmail.com > > > > > > -- > > > Thanks, > > > Nisha > > > > Hi Nisha, > > > > Some comments. > > > > Patch 0001 > > 1. Commit message > > > > from "two_phase enables slots." to "two_phase enabled slots" > > > > 2. > > + > > + <para> > > + When a subscription's > > + <link > > linkend="sql-createsubscription-params-with-two-phase"><literal>two_phase</literal></link> > > + is in the pending state, setting <literal>failover</literal> to > > true is not > > + permitted. Once > > > > The term "subscription's two_phase" sounds a bit vague here. > > Referring https://www.postgresql.org/docs/17/sql-createsubscription.html > > and https://www.postgresql.org/docs/current/catalog-pg-subscription.html > > two_phase is referred to both as the "internal state" as well as the > > "mode" of the subscription. > > > > Probably rephrase it to - "When a subscription's internal two_phase > > mode is in the pending state, setting failover to true is not > > permitted. See column subtwophasestate of pg_subscription to know the > > actual internal state of the two-phase mode" (with appropriate tags > > and reference links) > > > > There seems a potential muddling of terms two_phase VS two-phase. > > AFAIK "two_phase" (with the underscore) is the name of the > subscription option; it might also be the name of an internal > structure member or catalog column name. Furthermore, when you are > referring to the option name I think it ought to be double-quoted for > clarity. > > But, unless you are referring specifically to the option/field/column > then IMO it should say two-phase (with the dash). > e.g. The "two_phase" option enables two-phase mode. >
I agree with your suggestion to use two-phase when referring to the internal state of the subscription. I've updated the commit message and the respective doc (with reference link) accordingly in v19 patches. -- Thanks, Nisha
