Skip to content

Commit

Permalink
Disallow setting client_min_messages higher than ERROR.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tglsfdc committed Nov 8, 2018
1 parent 705d433 commit 3d360e2
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 47 deletions.
45 changes: 22 additions & 23 deletions doc/src/sgml/config.sgml
Original file line number Diff line number Diff line change
Expand Up @@ -5111,28 +5111,6 @@ local0.* /var/log/postgresql

<variablelist>

<varlistentry id="guc-client-min-messages" xreflabel="client_min_messages">
<term><varname>client_min_messages</varname> (<type>enum</type>)
<indexterm>
<primary><varname>client_min_messages</varname> configuration parameter</primary>
</indexterm>
</term>
<listitem>
<para>
Controls which message levels are sent to the client.
Valid values are <literal>DEBUG5</literal>,
<literal>DEBUG4</literal>, <literal>DEBUG3</literal>, <literal>DEBUG2</literal>,
<literal>DEBUG1</literal>, <literal>LOG</literal>, <literal>NOTICE</literal>,
<literal>WARNING</literal>, <literal>ERROR</literal>, <literal>FATAL</literal>,
and <literal>PANIC</literal>. Each level
includes all the levels that follow it. The later the level,
the fewer messages are sent. The default is
<literal>NOTICE</literal>. Note that <literal>LOG</literal> has a different
rank here than in <varname>log_min_messages</varname>.
</para>
</listitem>
</varlistentry>

<varlistentry id="guc-log-min-messages" xreflabel="log_min_messages">
<term><varname>log_min_messages</varname> (<type>enum</type>)
<indexterm>
Expand All @@ -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 <literal>WARNING</literal>. Note that
<literal>LOG</literal> has a different rank here than in
<varname>client_min_messages</varname>.
<xref linkend="guc-client-min-messages"/>.
Only superusers can change this setting.
</para>
</listitem>
Expand Down Expand Up @@ -6471,6 +6449,27 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
<title>Statement Behavior</title>
<variablelist>

<varlistentry id="guc-client-min-messages" xreflabel="client_min_messages">
<term><varname>client_min_messages</varname> (<type>enum</type>)
<indexterm>
<primary><varname>client_min_messages</varname> configuration parameter</primary>
</indexterm>
</term>
<listitem>
<para>
Controls which message levels are sent to the client.
Valid values are <literal>DEBUG5</literal>,
<literal>DEBUG4</literal>, <literal>DEBUG3</literal>, <literal>DEBUG2</literal>,
<literal>DEBUG1</literal>, <literal>LOG</literal>, <literal>NOTICE</literal>,
<literal>WARNING</literal>, and <literal>ERROR</literal>.
Each level includes all the levels that follow it. The later the level,
the fewer messages are sent. The default is
<literal>NOTICE</literal>. Note that <literal>LOG</literal> has a different
rank here than in <xref linkend="guc-log-min-messages"/>.
</para>
</listitem>
</varlistentry>

<varlistentry id="guc-search-path" xreflabel="search_path">
<term><varname>search_path</varname> (<type>string</type>)
<indexterm>
Expand Down
11 changes: 2 additions & 9 deletions src/backend/utils/error/elog.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions src/backend/utils/misc/guc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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}
};

Expand Down Expand Up @@ -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.")
Expand Down
21 changes: 10 additions & 11 deletions src/backend/utils/misc/postgresql.conf.sample
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 3d360e2

Please sign in to comment.