On Tue, Feb 9, 2016 at 7:26 PM, Thom Brown <
thom@linux.com> wrote:
>
> On 7 January 2016 at 05:24, Amit Kapila <
amit.kapila16@gmail.com> wrote:
> > On Fri, Dec 25, 2015 at 6:36 AM, Robert Haas <
robertmhaas@gmail.com> wrote:
> >>
> >> On Wed, Dec 23, 2015 at 1:16 AM, Amit Kapila <
amit.kapila16@gmail.com>
> >> wrote:
> >> >> >
> >> >> > Here procArrayGroupXid sounds like Xid at group level, how about
> >> >> > procArrayGroupMemberXid?
> >> >> > Find the patch with renamed variables for PGProc
> >> >> > (rename_pgproc_variables_v1.patch) attached with mail.
> >> >>
> >> >> I sort of hate to make these member names any longer, but I wonder if
> >> >> we should make it procArrayGroupClearXid etc.
> >> >
> >> > If we go by this suggestion, then the name will look like:
> >> > PGProc
> >> > {
> >> > ..
> >> > bool procArrayGroupClearXid, pg_atomic_uint32
> >> > procArrayGroupNextClearXid,
> >> > TransactionId procArrayGroupLatestXid;
> >> > ..
> >> >
> >> > PROC_HDR
> >> > {
> >> > ..
> >> > pg_atomic_uint32 procArrayGroupFirstClearXid;
> >> > ..
> >> > }
> >> >
> >> > I think whatever I sent in last patch were better. It seems to me it is
> >> > better to add some comments before variable names, so that anybody
> >> > referring them can understand better and I have added comments in
> >> > attached patch rename_pgproc_variables_v2.patch to explain the same.
> >>
> >> Well, I don't know. Anybody else have an opinion?
> >>
> >
> > It seems that either people don't have any opinion on this matter or they
> > are okay with either of the naming conventions being discussed. I think
> > specifying Member after procArrayGroup can help distinguishing which
> > variables are specific to the whole group and which are specific to a
> > particular member. I think that will be helpful for other places as well
> > if we use this technique to improve performance. Let me know what
> > you think about the same.
> >
> > I have verified that previous patches can be applied cleanly and passes
> > make check-world. To avoid confusion, I am attaching the latest
> > patches with this mail.
>
> Patches still apply 1 month later.
>
Thanks for verification!
>
> I don't really have an opinion on the variable naming. I guess they
> only need making longer if there's going to be some confusion about
> what they're for,
makes sense, that is the reason why I have added few comments
as well, but not sure if you are suggesting something else.
> but I'm guessing it's not a blocker here.
>
I also think so, but not sure what else is required here. The basic
idea of this rename_pgproc_variables_v2.patch is to rename
few variables in existing similar code, so that the main patch
group_update_clog can adapt those naming convention if required,
other than that I have handled all review comments raised in this
thread (mainly by Simon and Robert).
Is there anything, I can do to move this forward?