Skip to content

Commit 2407d48

Browse files
tglsfdctglsfdc
committed
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
1 parent b8db4c2 commit 2407d48

File tree

4 files changed

+51
-45
lines changed

4 files changed

+51
-45
lines changed

‎doc/src/sgml/config.sgml

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4056,28 +4056,6 @@ local0.* /var/log/postgresql
40564056

40574057
<variablelist>
40584058

4059-
<varlistentry id="guc-client-min-messages" xreflabel="client_min_messages">
4060-
<term><varname>client_min_messages</varname> (<type>enum</type>)
4061-
<indexterm>
4062-
<primary><varname>client_min_messages</> configuration parameter</primary>
4063-
</indexterm>
4064-
</term>
4065-
<listitem>
4066-
<para>
4067-
Controls which message levels are sent to the client.
4068-
Valid values are <literal>DEBUG5</>,
4069-
<literal>DEBUG4</>, <literal>DEBUG3</>, <literal>DEBUG2</>,
4070-
<literal>DEBUG1</>, <literal>LOG</>, <literal>NOTICE</>,
4071-
<literal>WARNING</>, <literal>ERROR</>, <literal>FATAL</>,
4072-
and <literal>PANIC</>. Each level
4073-
includes all the levels that follow it. The later the level,
4074-
the fewer messages are sent. The default is
4075-
<literal>NOTICE</>. Note that <literal>LOG</> has a different
4076-
rank here than in <varname>log_min_messages</>.
4077-
</para>
4078-
</listitem>
4079-
</varlistentry>
4080-
40814059
<varlistentry id="guc-log-min-messages" xreflabel="log_min_messages">
40824060
<term><varname>log_min_messages</varname> (<type>enum</type>)
40834061
<indexterm>
@@ -4095,7 +4073,7 @@ local0.* /var/log/postgresql
40954073
follow it. The later the level, the fewer messages are sent
40964074
to the log. The default is <literal>WARNING</>. Note that
40974075
<literal>LOG</> has a different rank here than in
4098-
<varname>client_min_messages</>.
4076+
<xref linkend="guc-client-min-messages">.
40994077
Only superusers can change this setting.
41004078
</para>
41014079
</listitem>
@@ -5343,6 +5321,27 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
53435321
<title>Statement Behavior</title>
53445322
<variablelist>
53455323

5324+
<varlistentry id="guc-client-min-messages" xreflabel="client_min_messages">
5325+
<term><varname>client_min_messages</varname> (<type>enum</type>)
5326+
<indexterm>
5327+
<primary><varname>client_min_messages</> configuration parameter</primary>
5328+
</indexterm>
5329+
</term>
5330+
<listitem>
5331+
<para>
5332+
Controls which message levels are sent to the client.
5333+
Valid values are <literal>DEBUG5</>,
5334+
<literal>DEBUG4</>, <literal>DEBUG3</>, <literal>DEBUG2</>,
5335+
<literal>DEBUG1</>, <literal>LOG</>, <literal>NOTICE</>,
5336+
<literal>WARNING</>, and <literal>ERROR</>.
5337+
Each level includes all the levels that follow it. The later the level,
5338+
the fewer messages are sent. The default is
5339+
<literal>NOTICE</>. Note that <literal>LOG</> has a different
5340+
rank here than in <xref linkend="guc-log-min-messages">.
5341+
</para>
5342+
</listitem>
5343+
</varlistentry>
5344+
53465345
<varlistentry id="guc-search-path" xreflabel="search_path">
53475346
<term><varname>search_path</varname> (<type>string</type>)
53485347
<indexterm>

‎src/backend/utils/error/elog.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -487,9 +487,7 @@ errfinish(int dummy,...)
487487
* progress, so that we can report the message before dying. (Without
488488
* this, pq_putmessage will refuse to send the message at all, which is
489489
* what we want for NOTICE messages, but not for fatal exits.) This hack
490-
* is necessary because of poor design of old-style copy protocol. Note
491-
* we must do this even if client is fool enough to have set
492-
* client_min_messages above FATAL, so don't look at output_to_client.
490+
* is necessary because of poor design of old-style copy protocol.
493491
*/
494492
if (elevel >= FATAL && whereToSendOutput == DestRemote)
495493
pq_endcopyout(true);
@@ -1690,12 +1688,7 @@ pg_re_throw(void)
16901688
else
16911689
edata->output_to_server = (FATAL >= log_min_messages);
16921690
if (whereToSendOutput == DestRemote)
1693-
{
1694-
if (ClientAuthInProgress)
1695-
edata->output_to_client = true;
1696-
else
1697-
edata->output_to_client = (FATAL >= client_min_messages);
1698-
}
1691+
edata->output_to_client = true;
16991692

17001693
/*
17011694
* We can use errfinish() for the rest, but we don't want it to call

‎src/backend/utils/misc/guc.c

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ static int syslog_facility = 0;
172172
static void assign_syslog_facility(int newval, void *extra);
173173
static void assign_syslog_ident(const char *newval, void *extra);
174174
static void assign_session_replication_role(int newval, void *extra);
175+
static bool check_client_min_messages(int *newval, void **extra, GucSource source);
175176
static bool check_temp_buffers(int *newval, void **extra, GucSource source);
176177
static bool check_phony_autocommit(bool *newval, void **extra, GucSource source);
177178
static bool check_debug_assertions(bool *newval, void **extra, GucSource source);
@@ -3276,14 +3277,14 @@ static struct config_enum ConfigureNamesEnum[] =
32763277
},
32773278

32783279
{
3279-
{"client_min_messages", PGC_USERSET, LOGGING_WHEN,
3280+
{"client_min_messages", PGC_USERSET, CLIENT_CONN_STATEMENT,
32803281
gettext_noop("Sets the message levels that are sent to the client."),
32813282
gettext_noop("Each level includes all the levels that follow it. The later"
32823283
" the level, the fewer messages are sent.")
32833284
},
32843285
&client_min_messages,
32853286
NOTICE, client_message_level_options,
3286-
NULL, NULL, NULL
3287+
check_client_min_messages, NULL, NULL
32873288
},
32883289

32893290
{
@@ -9138,6 +9139,20 @@ assign_session_replication_role(int newval, void *extra)
91389139
ResetPlanCache();
91399140
}
91409141

9142+
static bool
9143+
check_client_min_messages(int *newval, void **extra, GucSource source)
9144+
{
9145+
/*
9146+
* We disallow setting client_min_messages above ERROR, because not
9147+
* sending an ErrorResponse message for an error breaks the FE/BE
9148+
* protocol. However, for backwards compatibility, we still accept FATAL
9149+
* or PANIC as input values, and then adjust here.
9150+
*/
9151+
if (*newval > ERROR)
9152+
*newval = ERROR;
9153+
return true;
9154+
}
9155+
91419156
static bool
91429157
check_temp_buffers(int *newval, void **extra, GucSource source)
91439158
{

‎src/backend/utils/misc/postgresql.conf.sample

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -351,17 +351,6 @@
351351

352352
# - When to Log -
353353

354-
#client_min_messages = notice # values in order of decreasing detail:
355-
# debug5
356-
# debug4
357-
# debug3
358-
# debug2
359-
# debug1
360-
# log
361-
# notice
362-
# warning
363-
# error
364-
365354
#log_min_messages = warning # values in order of decreasing detail:
366355
# debug5
367356
# debug4
@@ -497,6 +486,16 @@
497486

498487
# - Statement Behavior -
499488

489+
#client_min_messages = notice # values in order of decreasing detail:
490+
# debug5
491+
# debug4
492+
# debug3
493+
# debug2
494+
# debug1
495+
# log
496+
# notice
497+
# warning
498+
# error
500499
#search_path = '"$user",public' # schema names
501500
#default_tablespace = '' # a tablespace name, '' uses the default
502501
#temp_tablespaces = '' # a list of tablespace names, '' uses

0 commit comments

Comments
 (0)