Disallow setting client_min_messages higher than ERROR.
authorTom Lane
Thu, 8 Nov 2018 22:33:26 +0000 (17:33 -0500)
committerTom Lane
Thu, 8 Nov 2018 22:33:26 +0000 (17:33 -0500)
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
src/backend/utils/error/elog.c
src/backend/utils/misc/guc.c
src/backend/utils/misc/postgresql.conf.sample

index 64a6074e1c078e2232bf09fb34e2ae448baaf931..34a2e295047702b84bd45378d4198c27b56d3a74 100644 (file)
@@ -4056,28 +4056,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)
       
@@ -4095,7 +4073,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
-        <varname>client_min_messages>.
+        <xref linkend="guc-client-min-messages">.
         Only superusers can change this setting.
        
       
@@ -5343,6 +5321,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)
       
index 10c90750c0c0f1c6a9f3d61ac4a729f55d8d4b81..41a516d7c9546cbd9d202a3817f82cd23cb15fd8 100644 (file)
@@ -487,9 +487,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);
@@ -1690,12 +1688,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
index 9c20fb84b63baac87e6388c0785ecf69f7739127..b7e1603f8bdfbd2e98c7dcb6afe1d121d9dee9d6 100644 (file)
@@ -172,6 +172,7 @@ static int  syslog_facility = 0;
 static void assign_syslog_facility(int newval, void *extra);
 static void assign_syslog_ident(const char *newval, void *extra);
 static void assign_session_replication_role(int newval, void *extra);
+static bool check_client_min_messages(int *newval, void **extra, GucSource source);
 static bool check_temp_buffers(int *newval, void **extra, GucSource source);
 static bool check_phony_autocommit(bool *newval, void **extra, GucSource source);
 static bool check_debug_assertions(bool *newval, void **extra, GucSource source);
@@ -3276,14 +3277,14 @@ 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.")
        },
        &client_min_messages,
        NOTICE, client_message_level_options,
-       NULL, NULL, NULL
+       check_client_min_messages, NULL, NULL
    },
 
    {
@@ -9138,6 +9139,20 @@ assign_session_replication_role(int newval, void *extra)
        ResetPlanCache();
 }
 
+static bool
+check_client_min_messages(int *newval, void **extra, GucSource source)
+{
+   /*
+    * We disallow setting client_min_messages above ERROR, because not
+    * sending an ErrorResponse message for an error breaks the FE/BE
+    * protocol.  However, for backwards compatibility, we still accept FATAL
+    * or PANIC as input values, and then adjust here.
+    */
+   if (*newval > ERROR)
+       *newval = ERROR;
+   return true;
+}
+
 static bool
 check_temp_buffers(int *newval, void **extra, GucSource source)
 {
index 51c9b01884b579623b6e4d04c7f92ed37820e735..93917b5edd00847e7d6b1a9a04d1bfadc3582b79 100644 (file)
 
 # - 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
 
 # - 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
 #default_tablespace = ''       # a tablespace name, '' uses the default
 #temp_tablespaces = ''         # a list of tablespace names, '' uses