From 99175141c9254318e5894ac30b9fdb622612acda Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 17 Sep 2020 12:52:18 -0400 Subject: [PATCH] Improve common/logging.c's support for multiple verbosity levels. Instead of hard-wiring specific verbosity levels into the option processing of client applications, invent pg_logging_increase_verbosity() and encourage clients to implement --verbose by calling that. Then, the common convention that more -v's gets you more verbosity just works. In particular, this allows resurrection of the debug-grade messages that have long existed in pg_dump and its siblings. They were unreachable before this commit due to lack of a way to select PG_LOG_DEBUG logging level. (It appears that they may have been unreachable for some time before common/logging.c was introduced, too, so I'm not specifically blaming cc8d41511 for the oversight. One reason for thinking that is that it's now apparent that _allocAH()'s message needs a null-pointer guard. Testing might have failed to reveal that before 96bf88d52.) Discussion: https://postgr.es/m/1173106.1600116625@sss.pgh.pa.us --- doc/src/sgml/ref/pg_dump.sgml | 2 ++ doc/src/sgml/ref/pg_dumpall.sgml | 4 +++- doc/src/sgml/ref/pg_restore.sgml | 7 ++++++- src/bin/pg_archivecleanup/pg_archivecleanup.c | 2 +- src/bin/pg_dump/pg_backup_archiver.c | 3 ++- src/bin/pg_dump/pg_dump.c | 2 +- src/bin/pg_dump/pg_dumpall.c | 2 +- src/bin/pg_dump/pg_restore.c | 2 +- src/bin/pg_rewind/pg_rewind.c | 2 +- src/bin/pgbench/pgbench.c | 2 +- src/common/logging.c | 18 ++++++++++++++++++ src/include/common/logging.h | 1 + 12 files changed, 38 insertions(+), 9 deletions(-) diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml index 0b2e2de87b..0ec99165c5 100644 --- a/doc/src/sgml/ref/pg_dump.sgml +++ b/doc/src/sgml/ref/pg_dump.sgml @@ -594,6 +594,8 @@ PostgreSQL documentation pg_dump to output detailed object comments and start/stop times to the dump file, and progress messages to standard error. + Repeating the option causes additional debug-level messages + to appear on standard error. diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml index 43abc530a0..5b514e4567 100644 --- a/doc/src/sgml/ref/pg_dumpall.sgml +++ b/doc/src/sgml/ref/pg_dumpall.sgml @@ -202,7 +202,9 @@ PostgreSQL documentation Specifies verbose mode. This will cause pg_dumpall to output start/stop times to the dump file, and progress messages to standard error. - It will also enable verbose output in pg_dump. + Repeating the option causes additional debug-level messages + to appear on standard error. + The option is also passed down to pg_dump. diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml index 27eab2f02a..e0d9d2ad64 100644 --- a/doc/src/sgml/ref/pg_restore.sgml +++ b/doc/src/sgml/ref/pg_restore.sgml @@ -483,7 +483,12 @@ PostgreSQL documentation - Specifies verbose mode. + Specifies verbose mode. This will cause + pg_restore to output detailed object + comments and start/stop times to the output file, and progress + messages to standard error. + Repeating the option causes additional debug-level messages + to appear on standard error. diff --git a/src/bin/pg_archivecleanup/pg_archivecleanup.c b/src/bin/pg_archivecleanup/pg_archivecleanup.c index e454bae767..12338e3bb2 100644 --- a/src/bin/pg_archivecleanup/pg_archivecleanup.c +++ b/src/bin/pg_archivecleanup/pg_archivecleanup.c @@ -302,7 +302,7 @@ main(int argc, char **argv) switch (c) { case 'd': /* Debug mode */ - pg_logging_set_level(PG_LOG_DEBUG); + pg_logging_increase_verbosity(); break; case 'n': /* Dry-Run mode */ dryrun = true; diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index c05a1fd6af..178b61d6cb 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -2278,7 +2278,8 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt, { ArchiveHandle *AH; - pg_log_debug("allocating AH for %s, format %d", FileSpec, fmt); + pg_log_debug("allocating AH for %s, format %d", + FileSpec ? FileSpec : "(stdio)", fmt); AH = (ArchiveHandle *) pg_malloc0(sizeof(ArchiveHandle)); diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 3406388872..320820165b 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -512,7 +512,7 @@ main(int argc, char **argv) case 'v': /* verbose */ g_verbose = true; - pg_logging_set_level(PG_LOG_INFO); + pg_logging_increase_verbosity(); break; case 'w': diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index 97d2b8dac1..219ca963c3 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -283,7 +283,7 @@ main(int argc, char *argv[]) case 'v': verbose = true; - pg_logging_set_level(PG_LOG_INFO); + pg_logging_increase_verbosity(); appendPQExpBufferStr(pgdumpopts, " -v"); break; diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c index 544ae3bc5c..eebf0d300b 100644 --- a/src/bin/pg_dump/pg_restore.c +++ b/src/bin/pg_dump/pg_restore.c @@ -245,7 +245,7 @@ main(int argc, char **argv) case 'v': /* verbose */ opts->verbose = 1; - pg_logging_set_level(PG_LOG_INFO); + pg_logging_increase_verbosity(); break; case 'w': diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 23fc749e44..0ec52cb032 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -181,7 +181,7 @@ main(int argc, char **argv) case 3: debug = true; - pg_logging_set_level(PG_LOG_DEBUG); + pg_logging_increase_verbosity(); break; case 'D': /* -D or --target-pgdata */ diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 332eabf637..663d7d292a 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -5522,7 +5522,7 @@ main(int argc, char **argv) pgport = pg_strdup(optarg); break; case 'd': - pg_logging_set_level(PG_LOG_DEBUG); + pg_logging_increase_verbosity(); break; case 'c': benchmarking_option_set = true; diff --git a/src/common/logging.c b/src/common/logging.c index 6a3a437a34..d9632fffc8 100644 --- a/src/common/logging.c +++ b/src/common/logging.c @@ -157,12 +157,30 @@ pg_logging_config(int new_flags) log_flags = new_flags; } +/* + * pg_logging_init sets the default log level to INFO. Programs that prefer + * a different default should use this to set it, immediately afterward. + */ void pg_logging_set_level(enum pg_log_level new_level) { __pg_log_level = new_level; } +/* + * Command line switches such as --verbose should invoke this. + */ +void +pg_logging_increase_verbosity(void) +{ + /* + * The enum values are chosen such that we have to decrease __pg_log_level + * in order to become more verbose. + */ + if (__pg_log_level > PG_LOG_NOTSET + 1) + __pg_log_level--; +} + void pg_logging_set_pre_callback(void (*cb) (void)) { diff --git a/src/include/common/logging.h b/src/include/common/logging.h index 028149c7a1..3205b8fef9 100644 --- a/src/include/common/logging.h +++ b/src/include/common/logging.h @@ -66,6 +66,7 @@ extern enum pg_log_level __pg_log_level; void pg_logging_init(const char *argv0); void pg_logging_config(int new_flags); void pg_logging_set_level(enum pg_log_level new_level); +void pg_logging_increase_verbosity(void); void pg_logging_set_pre_callback(void (*cb) (void)); void pg_logging_set_locus_callback(void (*cb) (const char **filename, uint64 *lineno));