On Thu, Sep 21, 2023 at 5:36 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Thu, Sep 21, 2023 at 9:34 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
> >
> > On Thu, Sep 21, 2023 at 08:14:23AM +0900, Michael Paquier wrote:
> > > On Thu, Sep 21, 2023 at 09:01:01AM +1000, Peter Smith wrote:
> > >> One question -- the patch comment still says "Bumps catversion.", but
> > >> catversion.h change is missing from the v9 patch?
> > >
> > > Yeah, previous patches did that, but it is no big deal. My take is
> > > that it is a good practice to never do a catversion bump in posted
> > > patches, and that it is equally a good practice from Nathan to be
> > > reminded about that with the addition of a note in the commit message
> > > of the patch posted.
> >
> > Right, I'll take care of it before committing. I'm trying to make sure I
> > don't forget. :)
>
> OK, all good.
>
> ~~~
>
> This is a bit of a late entry, but looking at the PG DOCS, I felt it
> might be simpler if we don't always refer to every other worker type
> when explaining NULLs. The descriptions are already bigger than they
> need to be, and if more types ever get added they will keep growing.
>
> ~
>
> BEFORE
> leader_pid integer
> Process ID of the leader apply worker if this process is a parallel
> apply worker; NULL if this process is a leader apply worker or a table
> synchronization worker
>
> SUGGESTION
> leader_pid integer
> Process ID of the leader apply worker; NULL if this process is not a
> parallel apply worker
>
> ~
>
> BEFORE
> relid oid
> OID of the relation that the worker is synchronizing; NULL for the
> leader apply worker and parallel apply workers
>
> SUGGESTION
> relid oid
> OID of the relation being synchronized; NULL if this process is not a
> table synchronization worker
>
I find the current descriptions better than the proposed. But I am not
opposed to your proposal if others are okay with it. Personally, I
feel even if we want to change these descriptions, we can do it as a
separate patch.
--
With Regards,
Amit Kapila.