At Sun, 19 Apr 2020 16:22:26 +0200, Julien Rouhaud <rjuju123@gmail.com> wrote in
> Hi Justin,
>
> Thanks for the review!
>
> On Sat, Apr 18, 2020 at 10:41 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> >
> > Should capitalize at least the non-text one ? And maybe the text one for
> > consistency ?
> >
> > + ExplainPropertyInteger("WAL fpw", NULL,
>
> I think we should keep both version consistent, whether lower or upper
> case. The uppercase version is probably more correct, but it's a
> little bit weird to have it being the only upper case label in all
> output, so I kept it lower case.
One space follwed by an acronym looks perfect. I'd prefer capital
letters but small-letters also works well.
> > And add the acronym to the docs:
> >
> > $ git grep 'full page' '*/explain.sgml'
> > doc/src/sgml/ref/explain.sgml: number of records, number of full page writes and amount of WAL bytes
> >
> > "..full page writes (FPW).."
>
> Indeed! Fixed (using lowercase to match current output).
I searched through the documentation and AFAICS most of occurances of
"full page" are follwed by "image" and full_page_writes is used only
as the parameter name.
I'm fine with fpw as the acronym, but "fpw means the number of full
page images" looks odd..
> > Should we also change vacuumlazy.c for consistency ?
> >
> > + _("WAL usage: %ld records, %ld full page writes, "
> > + UINT64_FORMAT " bytes"),
>
> I don't think this one should be changed, vacuumlazy output is already
> entirely different, and is way more verbose so keeping it as is makes
> sense to me.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
