"Unified logging system" breaks access to pg_dump debug outputs
pg_dump et al have some low-level debug log messages that commit
cc8d41511 converted to pg_log_debug() calls, replacing the previous
one-off logging verbosity system that was there. However, these
calls are dead code as things stand, because there is no way to set
__pg_log_level high enough to get them to print.
I propose the attached minimal patch to restore the previous
functionality.
Alternatively, we might consider inventing an additional logging.c
function pg_logging_increase_level() with the obvious semantics, and
make the various programs just call that when they see a -v switch.
That would be a slightly bigger patch, but it would more easily support
programs with a range of useful verbosities, so maybe that's a better
idea.
In a quick look around, I could not find any other unreachable
pg_log_debug calls.
(Note: it seems possible that the theoretical multiple verbosity
levels in pg_dump were already broken before cc8d41511, because
right offhand I do not see any code that that removed that would
have allowed invoking the higher levels either. Nonetheless, there
is no point in carrying dead code --- and these messages *are*
of some interest. I discovered this problem while trying to
debug parallel pg_restore behavior just now, and wondering
why "-v -v" didn't produce the messages I saw in the source code.)
regards, tom lane
Attachments:
restore-multiple-log-levels-in-pg_dump-1.patchtext/x-diff; charset=us-ascii; name=restore-multiple-log-levels-in-pg_dump-1.patchDownload+12-3
I wrote:
Alternatively, we might consider inventing an additional logging.c
function pg_logging_increase_level() with the obvious semantics, and
make the various programs just call that when they see a -v switch.
That would be a slightly bigger patch, but it would more easily support
programs with a range of useful verbosities, so maybe that's a better
idea.
After further thought, I concluded that's a clearly superior solution,
so 0001 attached does it like that. After noting that the enum values
are in the opposite direction from how I thought they went, I realized
that "increase_level" might be a bit ambiguous, so I now propose to
call it pg_logging_increase_verbosity.
(Note: it seems possible that the theoretical multiple verbosity
levels in pg_dump were already broken before cc8d41511, because
right offhand I do not see any code that that removed that would
have allowed invoking the higher levels either.
Closer inspection says this was almost certainly true, because
I discovered that pg_dump -v -v crashes if you don't specify
an output filename :-(. So this has probably been unreachable
at least since we went over to using our own snprintf always;
before that, depending on platform, it might've been fine.
So we also need 0002 attached to fix that.
regards, tom lane
On 2020-Sep-15, Tom Lane wrote:
I wrote:
Alternatively, we might consider inventing an additional logging.c
function pg_logging_increase_level() with the obvious semantics, and
make the various programs just call that when they see a -v switch.
That would be a slightly bigger patch, but it would more easily support
programs with a range of useful verbosities, so maybe that's a better
idea.After further thought, I concluded that's a clearly superior solution,
so 0001 attached does it like that. After noting that the enum values
are in the opposite direction from how I thought they went, I realized
that "increase_level" might be a bit ambiguous, so I now propose to
call it pg_logging_increase_verbosity.
I like this better too ... I've wished for extra-verbose messages from
pg_dump in the past, and this now allows me to add something should I
want them again.
(Note: it seems possible that the theoretical multiple verbosity
levels in pg_dump were already broken before cc8d41511, because
right offhand I do not see any code that that removed that would
have allowed invoking the higher levels either.Closer inspection says this was almost certainly true, because
I discovered that pg_dump -v -v crashes if you don't specify
an output filename :-(. So this has probably been unreachable
at least since we went over to using our own snprintf always;
before that, depending on platform, it might've been fine.
So we also need 0002 attached to fix that.
Ugh.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
On 2020-Sep-15, Tom Lane wrote:
After further thought, I concluded that's a clearly superior solution,
so 0001 attached does it like that. After noting that the enum values
are in the opposite direction from how I thought they went, I realized
that "increase_level" might be a bit ambiguous, so I now propose to
call it pg_logging_increase_verbosity.
I like this better too ... I've wished for extra-verbose messages from
pg_dump in the past, and this now allows me to add something should I
want them again.
Pushed, thanks for reviewing.
regards, tom lane