Hi Justin,

Thanks for the review!

On Sat, Apr 18, 2020 at 10:41 PM Justin Pryzby <pry...@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.

> 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).

> 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.
diff --git a/doc/src/sgml/ref/explain.sgml b/doc/src/sgml/ref/explain.sgml
index aedd70a6ad..e5b50d3790 100644
--- a/doc/src/sgml/ref/explain.sgml
+++ b/doc/src/sgml/ref/explain.sgml
@@ -198,8 +198,8 @@ ROLLBACK;
     <listitem>
      <para>
       Include information on WAL record generation. Specifically, include the
-      number of records, number of full page writes and amount of WAL bytes
-      generated.  In text format, only non-zero values are printed.  This
+      number of records, number of full page writes (fpw) and amount of WAL
+      bytes generated.  In text format, only non-zero values are printed.  This
       parameter may only be used when <literal>ANALYZE</literal> is also
       enabled.  It defaults to <literal>FALSE</literal>.
      </para>
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 7ae6131676..9cc1b13b76 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3350,13 +3350,13 @@ show_wal_usage(ExplainState *es, const WalUsage *usage)
 			appendStringInfoString(es->str, "WAL:");
 
 			if (usage->wal_records > 0)
-				appendStringInfo(es->str, "  records=%ld",
+				appendStringInfo(es->str, " records=%ld",
 								 usage->wal_records);
 			if (usage->wal_fpw > 0)
-				appendStringInfo(es->str, "  full page writes=%ld",
+				appendStringInfo(es->str, " fpw=%ld",
 								 usage->wal_fpw);
 			if (usage->wal_bytes > 0)
-				appendStringInfo(es->str, "  bytes=" UINT64_FORMAT,
+				appendStringInfo(es->str, " bytes=" UINT64_FORMAT,
 								 usage->wal_bytes);
 			appendStringInfoChar(es->str, '\n');
 		}
@@ -3365,7 +3365,7 @@ show_wal_usage(ExplainState *es, const WalUsage *usage)
 	{
 		ExplainPropertyInteger("WAL records", NULL,
 							   usage->wal_records, es);
-		ExplainPropertyInteger("WAL full page writes", NULL,
+		ExplainPropertyInteger("WAL fpw", NULL,
 							   usage->wal_fpw, es);
 		ExplainPropertyUInteger("WAL bytes", NULL,
 								usage->wal_bytes, es);

Reply via email to