On Thu, 2024-03-07 at 08:30 +0100, Laurenz Albe wrote:
> On Wed, 2024-03-06 at 17:33 -0500, Isaac Morland wrote:
> > I have two questions about this:
> > 
> > First, can it be done per role? If I have a particular application which is
> > constantly throwing some particular error, I might want to suppress it, but
> > not suppress the same error occasionally coming from another application.
> > I see ALTER DATABASE name SET configuration_parameter … as being useful 
> > here,
> > but often multiple applications share a database.
> > 
> > Second, where can this setting be adjusted? Can any session turn off logging
> > of arbitrary sets of sqlstates resulting from its queries? It feels to me
> > like that might allow security problems to be hidden. Specifically, the 
> > first
> > thing an SQL injection might do would be to turn off logging of important
> > error states, then proceed to try various nefarious things.
> 
> I was envisioning the parameter to be like other logging parameters, for
> example "log_statement":  only superusers can set the parameter or GRANT
> that privilege to others.  Also, a superuser could use ALTER ROLE to set
> the parameter for all sessions by that role.
> 
> > It seems to me the above questions interact; an answer to the first might be
> > "ALTER ROLE role_specification SET configuration_parameter", but I think 
> > that
> > would allow roles to change their own settings, contrary to the concern
> > raised by the second question.
> 
> If a superuser sets "log_statement" on a role, that role cannot undo or change
> the setting.  That's just how I plan to implement the new parameter.

Here is a patch that implements this.

I went with "log_suppress_errcodes", since the term "error code" is used
throughout our documentation.

The initial value is 23505,3D000,3F000,42601,42704,42883,42P01,57P03

Yours,
Laurenz Albe
From 26465dafe4bcd2aea38893609833337d2a863b66 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.a...@cybertec.at>
Date: Sat, 9 Mar 2024 13:59:55 +0100
Subject: [PATCH v1] Add parameter log_suppress_errcodes

The parameter contains a list of SQLSTATEs for which
FATAL and ERROR messages are not logged.  This is to
suppress messages that are of little interest to the
database administrator, but tend to clutter the log.

Author: Laurenz Albe
Discussion: https://postgr.es/m/408f399e7de1416c47bab7e260327ed5ad92838c.camel%40cybertec.at
---
 doc/src/sgml/config.sgml                      |  34 +++++
 src/backend/utils/error/elog.c                | 118 ++++++++++++++++++
 src/backend/utils/misc/guc_tables.c           |  11 ++
 src/backend/utils/misc/postgresql.conf.sample |   4 +
 src/include/pg_config_manual.h                |  10 ++
 src/include/utils/guc.h                       |   1 +
 src/include/utils/guc_hooks.h                 |   2 +
 7 files changed, 180 insertions(+)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 43b1a132a2..f9487de06f 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6850,6 +6850,40 @@ local0.*    /var/log/postgresql
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-log-suppress-errcodes" xreflabel="log_suppress_errcodes">
+      <term><varname>log_suppress_errcodes</varname> (<type>string</type>)
+      <indexterm>
+       <primary><varname>log_suppress_errcodes</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Causes <literal>ERROR</literal> and <literal>FATAL</literal> messages
+        with certain error codes to be excluded from the log.
+        The value is a comma-separated list of five-character error codes as
+        listed in <xref linkend="errcodes-appendix"/>.  An error code that
+        represents a class of errors (ends with three zeros) suppresses logging
+        of all error codes within that class.  For example, the entry
+        <literal>08000</literal> (<literal>connection_exception</literal>)
+        would suppress an error with code <literal>08P01</literal>
+        (<literal>protocol_violation</literal>).  The default setting is
+        <literal>23505,3D000,3F000,42601,42704,42883,42P01,57P03</literal>.
+        Only superusers and users with the appropriate <literal>SET</literal>
+        privilege can change this setting.
+       </para>
+
+       <para>
+        This setting is useful to exclude error messages from the log that are
+        frequent but irrelevant.  That makes it easier to spot relevant
+        messages in the log and keeps log files from growing too big.  For
+        example, if you have a monitoring system that regularly establishes a
+        TCP connection to the server without sending a correct startup packet,
+        you could suppress the protocol violation errors by adding error code
+        <literal>08P01</literal> to the list.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-log-min-duration-statement" xreflabel="log_min_duration_statement">
       <term><varname>log_min_duration_statement</varname> (<type>integer</type>)
       <indexterm>
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index c9719f358b..4b38adfd4d 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -112,12 +112,16 @@ int			Log_error_verbosity = PGERROR_DEFAULT;
 char	   *Log_line_prefix = NULL; /* format for extra log line info */
 int			Log_destination = LOG_DESTINATION_STDERR;
 char	   *Log_destination_string = NULL;
+char	   *log_suppress_errcodes = NULL;
 bool		syslog_sequence_numbers = true;
 bool		syslog_split_messages = true;
 
 /* Processed form of backtrace_functions GUC */
 static char *backtrace_function_list;
 
+/* Processed form form of log_suppress_errcodes (zero-terminated array) */
+static int *suppressed_errcodes;
+
 #ifdef HAVE_SYSLOG
 
 /*
@@ -866,6 +870,27 @@ errcode(int sqlerrcode)
 
 	edata->sqlerrcode = sqlerrcode;
 
+	/*
+	 * ERROR and FATAL messages with codes in log_suppress_errcodes don't get
+	 * logged.
+	 */
+	if ((edata->elevel == ERROR ||
+		 edata->elevel == FATAL) &&
+		suppressed_errcodes != NULL)
+	{
+		int *state;
+
+		for (state = suppressed_errcodes; *state != 0; state++)
+			/* error categories match all error codes in the category */
+			if (sqlerrcode == *state ||
+				(ERRCODE_IS_CATEGORY(*state) &&
+				 ERRCODE_TO_CATEGORY(sqlerrcode) == *state))
+			{
+				edata->output_to_server = false;
+				break;
+			}
+	}
+
 	return 0;					/* return value does not matter */
 }
 
@@ -2258,6 +2283,99 @@ assign_log_destination(const char *newval, void *extra)
 	Log_destination = *((int *) extra);
 }
 
+/*
+ * GUC check_hook for log_suppress_errcodes
+ */
+bool
+check_log_suppress_errcodes(char **newval, void **extra, GucSource source)
+{
+	/* SplitIdentifierString modifies the string */
+	char *new_copy = pstrdup(*newval);
+	int listlen;
+	int *statelist = NULL;
+	int index = 0;
+	List *states;
+	ListCell *c;
+
+	if (!SplitIdentifierString(new_copy, ',', &states))
+	{
+		GUC_check_errdetail("List syntax is invalid.");
+		goto failed;
+	}
+
+	listlen = list_length(states);
+	statelist = MemoryContextAlloc(TopMemoryContext,
+								   sizeof(int) * (listlen + 1));
+
+	/* check all error codes for validity and compile them into statelist */
+	foreach(c, states)
+	{
+		char *state = lfirst(c);
+		char *p;
+		int errcode;
+
+		if (strlen(state) != 5)
+		{
+			GUC_check_errdetail("error codes must have 5 characters.");
+			goto failed;
+		}
+
+		/*
+		 * Check the the values are alphanumeric and convert them to upper case
+		 * (SplitIdentifierString converted them to lower case).
+		 */
+		for (p = state; *p != '\0'; p++)
+			if (*p >= 'a' && *p <= 'z')
+				*p += 'A' - 'a';
+			else if (*p < '0' || *p > '9')
+			{
+				GUC_check_errdetail("error codes can only contain digits and ASCII letters.");
+				goto failed;
+			}
+
+		errcode = MAKE_SQLSTATE(state[0], state[1], state[2], state[3], state[4]);
+		/* ignore 0: it cannot be an error code, and we use it to end the list */
+		if (errcode == ERRCODE_SUCCESSFUL_COMPLETION)
+			continue;
+
+		statelist[index++] = errcode;
+	}
+	statelist[index] = 0;
+
+	list_free(states);
+	pfree(new_copy);
+
+	*extra = statelist;
+	return true;
+
+failed:
+	list_free(states);
+	pfree(new_copy);
+	if (statelist)
+		pfree(statelist);
+	return false;
+}
+
+/*
+ * GUC assign_hook for log_suppress_errcodes
+ */
+void
+assign_log_suppress_errcodes(const char *newval, void *extra)
+{
+	/* free the memory for the old entries */
+	if (suppressed_errcodes != NULL)
+		pfree(suppressed_errcodes);
+
+	/* store NULL instead of an empty array for performance */
+	if (*(int *)extra == 0)
+	{
+		pfree(extra);
+		suppressed_errcodes = NULL;
+	}
+	else
+		suppressed_errcodes = extra;
+}
+
 /*
  * GUC assign_hook for syslog_ident
  */
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 93ded31ed9..9dbb30fd33 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -4466,6 +4466,17 @@ struct config_string ConfigureNamesString[] =
 		check_canonical_path, NULL, NULL
 	},
 
+	{
+		{"log_suppress_errcodes", PGC_SUSET, LOGGING_WHEN,
+			gettext_noop("ERROR and FATAL messages with these error codes don't get logged."),
+			NULL,
+			GUC_LIST_INPUT
+		},
+		&log_suppress_errcodes,
+		DEFAULT_LOG_SUPPRESS_ERRCODES,
+		check_log_suppress_errcodes, assign_log_suppress_errcodes, NULL
+	},
+
 	{
 		{"ssl_library", PGC_INTERNAL, PRESET_OPTIONS,
 			gettext_noop("Shows the name of the SSL library."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index edcc0282b2..838c24eff9 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -536,6 +536,10 @@
 					#   fatal
 					#   panic (effectively off)
 
+#log_suppress_errcodes = '23505,3D000,3F000,42601,42704,42883,42P01,57P03'
+					# FATAL and ERROR messages with these error codes
+					# are not logged
+
 #log_min_duration_statement = -1	# -1 is disabled, 0 logs all statements
 					# and their durations, > 0 logs only
 					# statements running at least this number
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index a512552182..91c7b02b30 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -216,6 +216,16 @@
  */
 #define DEFAULT_EVENT_SOURCE  "PostgreSQL"
 
+/*
+ * Default value for log_suppress_errcodes.  ERROR or FATAL messages with
+ * these error codes are never logged.  Error classes (error codes ending with
+ * three zeros) match all error codes in the class.   The idea is to suppress
+ * messages that usually don't indicate a serious problem but tend to pollute
+ * the log file.
+ */
+
+#define DEFAULT_LOG_SUPPRESS_ERRCODES "23505,3D000,3F000,42601,42704,42883,42P01,57P03"
+
 /*
  * Assumed cache line size.  This doesn't affect correctness, but can be used
  * for low-level optimizations.  This is mostly used to pad various data
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 471d53da8f..a3499f1d14 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -258,6 +258,7 @@ extern PGDLLIMPORT bool log_duration;
 extern PGDLLIMPORT int log_parameter_max_length;
 extern PGDLLIMPORT int log_parameter_max_length_on_error;
 extern PGDLLIMPORT int log_min_error_statement;
+extern PGDLLIMPORT char *log_suppress_errcodes;
 extern PGDLLIMPORT int log_min_messages;
 extern PGDLLIMPORT int client_min_messages;
 extern PGDLLIMPORT int log_min_duration_sample;
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index c8a7aa9a11..a77ff24220 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -78,6 +78,8 @@ extern bool check_log_destination(char **newval, void **extra,
 extern void assign_log_destination(const char *newval, void *extra);
 extern const char *show_log_file_mode(void);
 extern bool check_log_stats(bool *newval, void **extra, GucSource source);
+extern bool check_log_suppress_errcodes(char **newval, void **extra, GucSource source);
+extern void assign_log_suppress_errcodes(const char *newval, void *extra);
 extern bool check_log_timezone(char **newval, void **extra, GucSource source);
 extern void assign_log_timezone(const char *newval, void *extra);
 extern const char *show_log_timezone(void);
-- 
2.44.0

Reply via email to