Skip to content

Commit c09daa9

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 21c9e49 commit c09daa9

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
@@ -4799,28 +4799,6 @@ local0.* /var/log/postgresql
47994799

48004800
<variablelist>
48014801

4802-
<varlistentry id="guc-client-min-messages" xreflabel="client_min_messages">
4803-
<term><varname>client_min_messages</varname> (<type>enum</type>)
4804-
<indexterm>
4805-
<primary><varname>client_min_messages</> configuration parameter</primary>
4806-
</indexterm>
4807-
</term>
4808-
<listitem>
4809-
<para>
4810-
Controls which message levels are sent to the client.
4811-
Valid values are <literal>DEBUG5</>,
4812-
<literal>DEBUG4</>, <literal>DEBUG3</>, <literal>DEBUG2</>,
4813-
<literal>DEBUG1</>, <literal>LOG</>, <literal>NOTICE</>,
4814-
<literal>WARNING</>, <literal>ERROR</>, <literal>FATAL</>,
4815-
and <literal>PANIC</>. Each level
4816-
includes all the levels that follow it. The later the level,
4817-
the fewer messages are sent. The default is
4818-
<literal>NOTICE</>. Note that <literal>LOG</> has a different
4819-
rank here than in <varname>log_min_messages</>.
4820-
</para>
4821-
</listitem>
4822-
</varlistentry>
4823-
48244802
<varlistentry id="guc-log-min-messages" xreflabel="log_min_messages">
48254803
<term><varname>log_min_messages</varname> (<type>enum</type>)
48264804
<indexterm>
@@ -4838,7 +4816,7 @@ local0.* /var/log/postgresql
48384816
follow it. The later the level, the fewer messages are sent
48394817
to the log. The default is <literal>WARNING</>. Note that
48404818
<literal>LOG</> has a different rank here than in
4841-
<varname>client_min_messages</>.
4819+
<xref linkend="guc-client-min-messages">.
48424820
Only superusers can change this setting.
48434821
</para>
48444822
</listitem>
@@ -6159,6 +6137,27 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
61596137
<title>Statement Behavior</title>
61606138
<variablelist>
61616139

6140+
<varlistentry id="guc-client-min-messages" xreflabel="client_min_messages">
6141+
<term><varname>client_min_messages</varname> (<type>enum</type>)
6142+
<indexterm>
6143+
<primary><varname>client_min_messages</> configuration parameter</primary>
6144+
</indexterm>
6145+
</term>
6146+
<listitem>
6147+
<para>
6148+
Controls which message levels are sent to the client.
6149+
Valid values are <literal>DEBUG5</>,
6150+
<literal>DEBUG4</>, <literal>DEBUG3</>, <literal>DEBUG2</>,
6151+
<literal>DEBUG1</>, <literal>LOG</>, <literal>NOTICE</>,
6152+
<literal>WARNING</>, and <literal>ERROR</>.
6153+
Each level includes all the levels that follow it. The later the level,
6154+
the fewer messages are sent. The default is
6155+
<literal>NOTICE</>. Note that <literal>LOG</> has a different
6156+
rank here than in <xref linkend="guc-log-min-messages">.
6157+
</para>
6158+
</listitem>
6159+
</varlistentry>
6160+
61626161
<varlistentry id="guc-search-path" xreflabel="search_path">
61636162
<term><varname>search_path</varname> (<type>string</type>)
61646163
<indexterm>

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

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -472,9 +472,7 @@ errfinish(int dummy,...)
472472
* progress, so that we can report the message before dying. (Without
473473
* this, pq_putmessage will refuse to send the message at all, which is
474474
* what we want for NOTICE messages, but not for fatal exits.) This hack
475-
* is necessary because of poor design of old-style copy protocol. Note
476-
* we must do this even if client is fool enough to have set
477-
* client_min_messages above FATAL, so don't look at output_to_client.
475+
* is necessary because of poor design of old-style copy protocol.
478476
*/
479477
if (elevel >= FATAL && whereToSendOutput == DestRemote)
480478
pq_endcopyout(true);
@@ -1758,12 +1756,7 @@ pg_re_throw(void)
17581756
else
17591757
edata->output_to_server = (FATAL >= log_min_messages);
17601758
if (whereToSendOutput == DestRemote)
1761-
{
1762-
if (ClientAuthInProgress)
1763-
edata->output_to_client = true;
1764-
else
1765-
edata->output_to_client = (FATAL >= client_min_messages);
1766-
}
1759+
edata->output_to_client = true;
17671760

17681761
/*
17691762
* 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
@@ -163,6 +163,7 @@ static int syslog_facility = 0;
163163
static void assign_syslog_facility(int newval, void *extra);
164164
static void assign_syslog_ident(const char *newval, void *extra);
165165
static void assign_session_replication_role(int newval, void *extra);
166+
static bool check_client_min_messages(int *newval, void **extra, GucSource source);
166167
static bool check_temp_buffers(int *newval, void **extra, GucSource source);
167168
static bool check_bonjour(bool *newval, void **extra, GucSource source);
168169
static bool check_ssl(bool *newval, void **extra, GucSource source);
@@ -3679,14 +3680,14 @@ static struct config_enum ConfigureNamesEnum[] =
36793680
},
36803681

36813682
{
3682-
{"client_min_messages", PGC_USERSET, LOGGING_WHEN,
3683+
{"client_min_messages", PGC_USERSET, CLIENT_CONN_STATEMENT,
36833684
gettext_noop("Sets the message levels that are sent to the client."),
36843685
gettext_noop("Each level includes all the levels that follow it. The later"
36853686
" the level, the fewer messages are sent.")
36863687
},
36873688
&client_min_messages,
36883689
NOTICE, client_message_level_options,
3689-
NULL, NULL, NULL
3690+
check_client_min_messages, NULL, NULL
36903691
},
36913692

36923693
{
@@ -10162,6 +10163,20 @@ assign_session_replication_role(int newval, void *extra)
1016210163
ResetPlanCache();
1016310164
}
1016410165

10166+
static bool
10167+
check_client_min_messages(int *newval, void **extra, GucSource source)
10168+
{
10169+
/*
10170+
* We disallow setting client_min_messages above ERROR, because not
10171+
* sending an ErrorResponse message for an error breaks the FE/BE
10172+
* protocol. However, for backwards compatibility, we still accept FATAL
10173+
* or PANIC as input values, and then adjust here.
10174+
*/
10175+
if (*newval > ERROR)
10176+
*newval = ERROR;
10177+
return true;
10178+
}
10179+
1016510180
static bool
1016610181
check_temp_buffers(int *newval, void **extra, GucSource source)
1016710182
{

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

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

386386
# - When to Log -
387387

388-
#client_min_messages = notice # values in order of decreasing detail:
389-
# debug5
390-
# debug4
391-
# debug3
392-
# debug2
393-
# debug1
394-
# log
395-
# notice
396-
# warning
397-
# error
398-
399388
#log_min_messages = warning # values in order of decreasing detail:
400389
# debug5
401390
# debug4
@@ -539,6 +528,16 @@
539528

540529
# - Statement Behavior -
541530

531+
#client_min_messages = notice # values in order of decreasing detail:
532+
# debug5
533+
# debug4
534+
# debug3
535+
# debug2
536+
# debug1
537+
# log
538+
# notice
539+
# warning
540+
# error
542541
#search_path = '"$user", public' # schema names
543542
#default_tablespace = '' # a tablespace name, '' uses the default
544543
#temp_tablespaces = '' # a list of tablespace names, '' uses

0 commit comments

Comments
 (0)