PG_COLOR not mentioned in docs of vacuumlo, oid2name and pgbench

On 4 Mar 2020, at 08:54, Michael Paquier <michael@paquier.xyz> wrote:

All the tools mentioned in $subject have been switched recently to use
the central logging infrastructure, which means that they have gained
coloring output. However we (mostly I) forgot to update the docs.

+1 on updating the docs with PG_COLOR for these.

Attached is a patch to fix this issue. Please let me know if there
are comments and/or objections.

+   color in diagnostics messages.  Possible values are
+   <literal>always</literal>, <literal>auto</literal>,
+   <literal>never</literal>.

Not being a native english speaker, I might have it backwards, but I find lists
of values in a sentence like this to be easier to read when the final value is
separated by a conjunction like:

<item 1>, <item 2>, .. , <item n-1> and <item n>

cheers ./daniel

On Wed, Mar 4, 2020 at 8:54 AM Michael Paquier <michael@paquier.xyz> wrote:

Attached is a patch to fix this issue. Please let me know if there
are comments and/or objections.

I think there are a couple tools missing: pg_archivecleanup, pg_ctl,
pg_test_fsync and pg_upgrade. pg_regress also, but there is nothing to do
in the documentation with it.

Regards,

Juan José Santamaría Flecha

Bonjour Michaël,

All the tools mentioned in $subject have been switched recently to use
the central logging infrastructure, which means that they have gained
coloring output. However we (mostly I) forgot to update the docs.

Attached is a patch to fix this issue. Please let me know if there
are comments and/or objections.

No objection. I did not know there was such a thing…

Maybe a more detailed explanation about PG_COLOR could be stored
somewhere, and all affected tools could link to it? Or not.

For "pgbench", you could also add the standard sentence that it uses libpq
environment variables, as it is also missing?

--
Fabien.

On Wed, Mar 04, 2020 at 10:12:23AM +0100, Daniel Gustafsson wrote:

+   color in diagnostics messages.  Possible values are
+   <literal>always</literal>, <literal>auto</literal>,
+   <literal>never</literal>.

Not being a native english speaker, I might have it backwards, but I find lists
of values in a sentence like this to be easier to read when the final value is
separated by a conjunction like:

<item 1>, <item 2>, .. , <item n-1> and <item n>

Point received. Your suggestion is more natural to me as well. Now,
all the existing docs don't follow that style so I chose consistency.
--
Michael

On Wed, Mar 04, 2020 at 10:22:26AM +0100, Juan José Santamaría Flecha wrote:

I think there are a couple tools missing: pg_archivecleanup, pg_ctl,
pg_test_fsync and pg_upgrade. pg_regress also, but there is nothing to do
in the documentation with it.

Indeed, true for pg_archivecleanup and pg_test_fsync, but not for
pg_ctl and pg_upgrade. The funny part about pg_ctl is that the
initialization is done for nothing, because nothing is actually logged
with the APIs of logging.c. pg_upgrade uses its own logging APIs,
which have nothing to do with logging.c.
--
Michael

On Wed, Mar 04, 2020 at 11:31:27AM +0100, Fabien COELHO wrote:

No objection. I did not know there was such a thing…

Maybe a more detailed explanation about PG_COLOR could be stored somewhere,
and all affected tools could link to it? Or not.

One argument against that position is that each tool may just handle a
subset of the full set available, and that some of the subsets may
partially intersect. Fun.

For "pgbench", you could also add the standard sentence that it uses libpq
environment variables, as it is also missing?

Yeah, that's true. Let's fix this part while on it.
--
Michael

On Wed, Mar 04, 2020 at 10:05:30PM +0900, Michael Paquier wrote:

On Wed, Mar 04, 2020 at 11:31:27AM +0100, Fabien COELHO wrote:

For "pgbench", you could also add the standard sentence that it uses libpq
environment variables, as it is also missing?

Yeah, that's true. Let's fix this part while on it.

So, combining the feedback from Fabien, Juan and Daniel I am finishing
with the attached. Any thoughts?
--
Michael

On 5 Mar 2020, at 08:26, Michael Paquier <michael@paquier.xyz> wrote:

On Wed, Mar 04, 2020 at 10:05:30PM +0900, Michael Paquier wrote:

On Wed, Mar 04, 2020 at 11:31:27AM +0100, Fabien COELHO wrote:

For "pgbench", you could also add the standard sentence that it uses libpq
environment variables, as it is also missing?

Yeah, that's true. Let's fix this part while on it.

So, combining the feedback from Fabien, Juan and Daniel I am finishing
with the attached. Any thoughts?

LGTM

cheers ./daniel

On Thu, Mar 5, 2020 at 9:40 AM Daniel Gustafsson <daniel@yesql.se> wrote:

On 5 Mar 2020, at 08:26, Michael Paquier <michael@paquier.xyz> wrote:

So, combining the feedback from Fabien, Juan and Daniel I am finishing
with the attached. Any thoughts?

LGTM

+1

Regards

On Thu, Mar 05, 2020 at 10:09:31AM +0100, Juan José Santamaría Flecha wrote:

On Thu, Mar 5, 2020 at 9:40 AM Daniel Gustafsson <daniel@yesql.se> wrote:

LGTM

+1

Thanks to both of you for the reviews. Please note that I will
mention the business with pg_ctl and logging in a new thread and
remove the diff of pg_ctl.c from the previous patch, and that the doc
changes could be backpatched down to 12 for the relevant parts. The
documentation for PG_COLORS is still missing, but that's not new and I
think that we had better handle that case separately by creating a new
section in the docs. For now, let's wait a couple of days and see if
others have more thoughts to share about the doc patch of this thread.
--
Michael

On Sat, Mar 07, 2020 at 10:09:23AM +0900, Michael Paquier wrote:

Thanks to both of you for the reviews. Please note that I will
mention the business with pg_ctl and logging in a new thread and
remove the diff of pg_ctl.c from the previous patch, and that the doc
changes could be backpatched down to 12 for the relevant parts. The
documentation for PG_COLORS is still missing, but that's not new and I
think that we had better handle that case separately by creating a new
section in the docs. For now, let's wait a couple of days and see if
others have more thoughts to share about the doc patch of this thread.

Hearing nothing, done. The part about pgbench with PGHOST, PGUSER and
PGPORT could go further down, but it has been like that for years so I
did not bother.
--
Michael