From 3d360e20c9ae48a8139b4dd84f5a8bacc392d8e9 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 8 Nov 2018 17:33:25 -0500 Subject: [PATCH] Disallow setting client_min_messages higher than ERROR. Previously it was possible to set client_min_messages to FATAL or PANIC, which had the effect of suppressing transmission of regular ERROR messages to the client. Perhaps that seemed like a useful option in the past, but the trouble with it is that it breaks guarantees that are explicitly made in our FE/BE protocol spec about how a query cycle can end. While libpq and psql manage to cope with the omission, that's mostly because they are not very bright; client libraries that have more semantic knowledge are likely to get confused. Notably, pgODBC doesn't behave very sanely. Let's fix this by getting rid of the ability to set client_min_messages above ERROR. In HEAD, just remove the FATAL and PANIC options from the set of allowed enum values for client_min_messages. (This change also affects trace_recovery_messages, but that's OK since these aren't useful values for that variable either.) In the back branches, there was concern that rejecting these values might break applications that are explicitly setting things that way. I'm pretty skeptical of that argument, but accommodate it by accepting these values and then internally setting the variable to ERROR anyway. In all branches, this allows a couple of tiny simplifications in the logic in elog.c, so do that. Also respond to the point that was made that client_min_messages has exactly nothing to do with the server's logging behavior, and therefore does not belong in the "When To Log" subsection of the documentation. The "Statement Behavior" subsection is a better match, so move it there. Jonah Harris and Tom Lane Discussion: https://postgr.es/m/7809.1541521180@sss.pgh.pa.us Discussion: https://postgr.es/m/15479-ef0f4cc2fd995ca2@postgresql.org --- doc/src/sgml/config.sgml | 45 +++++++++---------- src/backend/utils/error/elog.c | 11 +---- src/backend/utils/misc/guc.c | 7 ++- src/backend/utils/misc/postgresql.conf.sample | 21 +++++---- 4 files changed, 37 insertions(+), 47 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index b8e32d765b..567d2246e8 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -5111,28 +5111,6 @@ local0.* /var/log/postgresql - - client_min_messages (enum) - - client_min_messages configuration parameter - - - - - Controls which message levels are sent to the client. - Valid values are DEBUG5, - DEBUG4, DEBUG3, DEBUG2, - DEBUG1, LOG, NOTICE, - WARNING, ERROR, FATAL, - and PANIC. Each level - includes all the levels that follow it. The later the level, - the fewer messages are sent. The default is - NOTICE. Note that LOG has a different - rank here than in log_min_messages. - - - - log_min_messages (enum) @@ -5150,7 +5128,7 @@ local0.* /var/log/postgresql follow it. The later the level, the fewer messages are sent to the log. The default is WARNING. Note that LOG has a different rank here than in - client_min_messages. + . Only superusers can change this setting. @@ -6471,6 +6449,27 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; Statement Behavior + + client_min_messages (enum) + + client_min_messages configuration parameter + + + + + Controls which message levels are sent to the client. + Valid values are DEBUG5, + DEBUG4, DEBUG3, DEBUG2, + DEBUG1, LOG, NOTICE, + WARNING, and ERROR. + Each level includes all the levels that follow it. The later the level, + the fewer messages are sent. The default is + NOTICE. Note that LOG has a different + rank here than in . + + + + search_path (string) diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index b9c11301ca..ca02aac4eb 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -469,9 +469,7 @@ errfinish(int dummy,...) * progress, so that we can report the message before dying. (Without * this, pq_putmessage will refuse to send the message at all, which is * what we want for NOTICE messages, but not for fatal exits.) This hack - * is necessary because of poor design of old-style copy protocol. Note - * we must do this even if client is fool enough to have set - * client_min_messages above FATAL, so don't look at output_to_client. + * is necessary because of poor design of old-style copy protocol. */ if (elevel >= FATAL && whereToSendOutput == DestRemote) pq_endcopyout(true); @@ -1747,12 +1745,7 @@ pg_re_throw(void) else edata->output_to_server = (FATAL >= log_min_messages); if (whereToSendOutput == DestRemote) - { - if (ClientAuthInProgress) - edata->output_to_client = true; - else - edata->output_to_client = (FATAL >= client_min_messages); - } + edata->output_to_client = true; /* * We can use errfinish() for the rest, but we don't want it to call diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index ce54828fbb..0327b295da 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -215,7 +215,8 @@ static const struct config_enum_entry bytea_output_options[] = { /* * We have different sets for client and server message level options because - * they sort slightly different (see "log" level) + * they sort slightly different (see "log" level), and because "fatal"/"panic" + * aren't sensible for client_min_messages. */ static const struct config_enum_entry client_message_level_options[] = { {"debug5", DEBUG5, false}, @@ -229,8 +230,6 @@ static const struct config_enum_entry client_message_level_options[] = { {"notice", NOTICE, false}, {"warning", WARNING, false}, {"error", ERROR, false}, - {"fatal", FATAL, true}, - {"panic", PANIC, true}, {NULL, 0, false} }; @@ -3924,7 +3923,7 @@ static struct config_enum ConfigureNamesEnum[] = }, { - {"client_min_messages", PGC_USERSET, LOGGING_WHEN, + {"client_min_messages", PGC_USERSET, CLIENT_CONN_STATEMENT, gettext_noop("Sets the message levels that are sent to the client."), gettext_noop("Each level includes all the levels that follow it. The later" " the level, the fewer messages are sent.") diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 4e61bc6521..3fe257c53f 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -406,17 +406,6 @@ # - When to Log - -#client_min_messages = notice # values in order of decreasing detail: - # debug5 - # debug4 - # debug3 - # debug2 - # debug1 - # log - # notice - # warning - # error - #log_min_messages = warning # values in order of decreasing detail: # debug5 # debug4 @@ -561,6 +550,16 @@ # - Statement Behavior - +#client_min_messages = notice # values in order of decreasing detail: + # debug5 + # debug4 + # debug3 + # debug2 + # debug1 + # log + # notice + # warning + # error #search_path = '"$user", public' # schema names #row_security = on #default_tablespace = '' # a tablespace name, '' uses the default -- 2.40.0