On Tue, Jan 14, 2014 at 12:22:30PM +0530, Jeevan Chalke wrote:
> On Mon, Jan 13, 2014 at 4:30 PM, Oskari Saarenmaa <o...@ohmu.fi> wrote:
> > On 13/01/14 10:26, Jeevan Chalke wrote:
> >
> > > 1. Documentation is missing and thus becomes difficult to understand
> > > what exactly you are trying to do. Or in other words, user will be
> > > uncertain about using it more efficiently.
> >
> > I figured I'd write documentation for this if it looks like a useful
> > feature which would be accepted for 9.4, but I guess it would've
> > helped to have a bit better description of this for the initial
> > submission as well.
> >
> >
> > > 2. Some more comments required. At each new function and
> > > specifically at get_sqlstate_error_level().
> >
> > Just after I submitted the patch I noticed that I had a placeholder
> > for comment about that function but never wrote the actual comment,
> > sorry about that.
> >
> > > 3. Please add test-case if possible.
> >
> > Sure.
> >
> > > 4. Some code part does not comply with PostgreSQL indentation style.
> > > (Can be ignored as it will pass through pg_indent, but better fix
> > > it).
> > >
> >
> > I'll try to fix this for v2.
> >
> > > 5. You have used ""XX000:warning," string to get maximum possible
> > > length of the valid sqlstate:level identifier. It's perfect, but
> > > small explanation about that will be good there. Also in future if
> > > we have any other error level which exceeds this, we need changes
> > > here too. Right ?
> >
> > Good point, I'll address this in v2.
> >
> > > I will look into this further. But please have your attention on above
> > > points.
> >
> > Thanks for the review!
>
> Since you are taking care of most of the points above. I will wait for v2
> patch. Till then marking "Waiting on Author".
Attached v2 of the patch which addresses the above points. I couldn't
figure out how to test log output, but at least the patch now tests that
it can set and show the log level.
Thanks,
Oskari
>From 213f647657f318141e3866087a17a863a0f322d9 Mon Sep 17 00:00:00 2001
From: Oskari Saarenmaa <o...@ohmu.fi>
Date: Tue, 14 Jan 2014 15:47:39 +0200
Subject: [PATCH] Filter error log statements by sqlstate
Allow the default log_min_error_statement to be overridden per sqlstate to
make it possible to filter out some error types while maintaining a low
log_min_error_statement or enable logging for some error types when the
default is to not log anything.
---
doc/src/sgml/config.sgml | 30 ++++++
src/backend/utils/error/elog.c | 220 +++++++++++++++++++++++++++++++++++++-
src/backend/utils/misc/guc.c | 14 ++-
src/include/utils/guc.h | 4 +
src/include/utils/guc_tables.h | 1 +
src/test/regress/expected/guc.out | 24 +++++
src/test/regress/sql/guc.sql | 8 ++
7 files changed, 298 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 0f2f2bf..73a58ad 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3743,6 +3743,36 @@ local0.* /var/log/postgresql
</listitem>
</varlistentry>
+ <varlistentry id="guc-log-sqlstate-error-statement"
xreflabel="log_sqlstate_error_statement">
+ <term><varname>log_sqlstate_error_statement</varname>
(<type>string</type>)</term>
+ <indexterm>
+ <primary><varname>log_sqlstate_error_statement</> configuration
parameter</primary>
+ </indexterm>
+ <listitem>
+ <para>
+ Controls which error types in SQL statements condition are recorded
+ in the server log. This overrides the global <xref
+ linkend="guc-log-min-messages"> per error type and can be used to
+ disable logging for certain error types and/or to enable logging for
+ other types.
+ </para>
+ <para>
+ The value must be a comma-separated list in the form
+ <literal>'ERRORCODE:LOGLEVEL,...'</literal>. For example, a setting
+ of <literal>'P0001:PANIC,22012:ERROR'</literal> would never log the
+ SQL statements for errors generated by the PL/pgSQL
+ <literal>RAISE</literal> statement but would always log the
+ statements causing division by zero errors.
+
+ See <xref linkend="errcodes-appendix"> for the list of valid error
+ codes and <xref linkend="guc-log-min-messages"> for valid log
+ levels.
+
+ Only superusers can change this setting.
+ </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>)</term>
<indexterm>
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 8705586..2e74fd5 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -74,7 +74,9 @@
#include "storage/ipc.h"
#include "storage/proc.h"
#include "tcop/tcopprot.h"
+#include "utils/builtins.h"
#include "utils/guc.h"
+#include "utils/guc_tables.h"
#include "utils/memutils.h"
#include "utils/ps_status.h"
@@ -111,6 +113,11 @@ char *Log_line_prefix = NULL; /*
format for extra log line info */
int Log_destination = LOG_DESTINATION_STDERR;
char *Log_destination_string = NULL;
+static uint64 *log_sqlstate_error_statement = NULL;
+static size_t log_sqlstate_error_statement_len = 0;
+
+static int get_sqlstate_error_level(int sqlstate);
+
#ifdef HAVE_SYSLOG
/*
@@ -2496,6 +2503,7 @@ static void
write_csvlog(ErrorData *edata)
{
StringInfoData buf;
+ int requested_log_level;
bool print_stmt = false;
/* static counter for line numbers */
@@ -2639,7 +2647,10 @@ write_csvlog(ErrorData *edata)
appendStringInfoChar(&buf, ',');
/* user query --- only reported if not disabled by the caller */
- if (is_log_level_output(edata->elevel, log_min_error_statement) &&
+ requested_log_level = get_sqlstate_error_level(edata->sqlerrcode);
+ if (requested_log_level < 0)
+ requested_log_level = log_min_error_statement;
+ if (is_log_level_output(edata->elevel, requested_log_level) &&
debug_query_string != NULL &&
!edata->hide_stmt)
print_stmt = true;
@@ -2712,6 +2723,7 @@ static void
send_message_to_server_log(ErrorData *edata)
{
StringInfoData buf;
+ int requested_log_level;
initStringInfo(&buf);
@@ -2796,7 +2808,10 @@ send_message_to_server_log(ErrorData *edata)
/*
* If the user wants the query that generated this error logged, do it.
*/
- if (is_log_level_output(edata->elevel, log_min_error_statement) &&
+ requested_log_level = get_sqlstate_error_level(edata->sqlerrcode);
+ if (requested_log_level < 0)
+ requested_log_level = log_min_error_statement;
+ if (is_log_level_output(edata->elevel, requested_log_level) &&
debug_query_string != NULL &&
!edata->hide_stmt)
{
@@ -3598,3 +3613,204 @@ trace_recovery(int trace_level)
return trace_level;
}
+
+
+/*
+ * Statement log error level can be overriden per sqlstate, the variable
+ * log_sqlstate_error_statement is a sorted array of
+ * log_sqlstate_error_statement_len uint64s where the high 32 bits contain
+ * the sqlstate and low 32 bits contain the required error level.
+ */
+
+/*
+ * get_sqlstate_error_level - perform a binary search for the requested
+ * state and return if it was found; return -1 if it was not found.
+ */
+static int
+get_sqlstate_error_level(int sqlstate)
+{
+ uint64 left = 0,
+ right = log_sqlstate_error_statement_len;
+
+ while (left < right)
+ {
+ uint64 middle = left + (right - left) / 2;
+ int m_sqlstate =
log_sqlstate_error_statement[middle] >> 32;
+
+ if (m_sqlstate == sqlstate)
+ return log_sqlstate_error_statement[middle] &
0xFFFFFFFF;
+ else if (m_sqlstate < sqlstate)
+ left = middle + 1;
+ else
+ right = middle;
+ }
+ return -1;
+}
+
+/*
+ * check_log_sqlstate_error - validate sqlstate:errorlevel lists and create
+ * sorted uint64 arrays from them; last occurance of each sqlstate is used.
+ */
+bool
+check_log_sqlstate_error(char **newval, void **extra, GucSource source)
+{
+ const struct config_enum_entry *enum_entry;
+ char *rawstring,
+ *new_newval,
+ *new_newval_end,
+ *vptr;
+ List *elemlist;
+ ListCell *l;
+ uint64 *new_array = NULL;
+ int i,
+ new_array_len = 0;
+
+ /* Need a modifiable copy of string */
+ rawstring = pstrdup(*newval);
+
+ /* Parse string into list of identifiers */
+ if (!SplitIdentifierString(rawstring, ',', &elemlist))
+ {
+ /* syntax error in list */
+ GUC_check_errdetail("List syntax is invalid.");
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+
+ /*
+ * GUC wants malloced results, allocate room for as many elements on the
+ * list plus one to hold the array size
+ */
+ new_array = (uint64 *) malloc(sizeof(uint64) * (list_length(elemlist) +
1));
+ if (!new_array)
+ {
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+
+ /* validate list and insert the results in a sorted array */
+ foreach(l, elemlist)
+ {
+ char *tok = lfirst(l),
+ *level_str = strchr(tok, ':');
+ int level = -1,
+ sqlstate;
+ uint64 value;
+
+ if (level_str != NULL && (level_str - tok) == 5)
+ {
+ for (enum_entry = server_message_level_options;
+ enum_entry && enum_entry->name;
+ enum_entry++)
+ {
+ if (pg_strcasecmp(enum_entry->name, level_str +
1) == 0)
+ {
+ level = enum_entry->val;
+ break;
+ }
+ }
+ }
+ if (level < 0)
+ {
+ GUC_check_errdetail("Invalid sqlstate error definition:
\"%s\".", tok);
+ new_array_len = -1;
+ break;
+ }
+ sqlstate = MAKE_SQLSTATE(pg_ascii_toupper(tok[0]),
pg_ascii_toupper(tok[1]),
+
pg_ascii_toupper(tok[2]), pg_ascii_toupper(tok[3]),
+
pg_ascii_toupper(tok[4]));
+ value = (((uint64) sqlstate) << 32) | ((uint64) level);
+
+ for (i = 0; i <= new_array_len; i++)
+ {
+ if (i == new_array_len)
+ {
+ new_array[++new_array_len] = value;
+ break;
+ }
+ else if (sqlstate == (int) (new_array[i + 1] >> 32))
+ {
+ new_array[i + 1] = value;
+ break;
+ }
+ else if (sqlstate < (int) (new_array[i + 1] >> 32))
+ {
+ memmove(&new_array[i + 2], &new_array[i + 1],
+ (new_array_len - i) *
sizeof(uint64));
+ ++new_array_len;
+ new_array[i + 1] = value;
+ break;
+ }
+ }
+ }
+
+ pfree(rawstring);
+ list_free(elemlist);
+
+ if (new_array_len < 0)
+ {
+ free(new_array);
+ return false;
+ }
+
+ /* store the length in the first field */
+ new_array[0] = new_array_len;
+
+ /*
+ * calculate the maximum length for the canonical string and
+ * allocate a buffer for it. sqlstate is always 5 characters and
+ * the longest error level string currently is 'warning'.
+ */
+ new_newval = (char *) malloc(strlen("XX000:warning,") * new_array_len +
1);
+ if (!new_newval)
+ {
+ free(new_array);
+ return false;
+ }
+
+ vptr = new_newval;
+ for (i = 1; i <= new_array_len; i++)
+ {
+ const char *level_str = "null";
+
+ for (enum_entry = server_message_level_options;
+ enum_entry && enum_entry->name;
+ enum_entry++)
+ {
+ if (enum_entry->val == (new_array[i] & 0xFFFFFFFF))
+ {
+ level_str = enum_entry->name;
+ break;
+ }
+ }
+
+ vptr += snprintf(vptr, new_newval_end - vptr, "%s%s:%s",
+ (i > 1) ? "," : "",
+ unpack_sql_state(new_array[i]
>> 32),
+ level_str);
+ }
+ *vptr = 0;
+
+ free(*newval);
+ *newval = new_newval;
+ *extra = new_array;
+
+ return true;
+}
+
+/*
+ * assign_log_sqlstate_error - take the array generated by
+ * check_log_sqlstate_error into use. 'extra' is an array of uint64s where
+ * the first element contains the list length and the remainder is
+ * sqlstate:errorlevel values.
+ */
+void
+assign_log_sqlstate_error(const char *newval, void *extra)
+{
+ uint64 *myextra = (uint64 *) extra;
+
+ log_sqlstate_error_statement_len = myextra[0];
+ log_sqlstate_error_statement = &myextra[1];
+}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 1217098..f6cbee5 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -245,7 +245,7 @@ static const struct config_enum_entry
client_message_level_options[] = {
{NULL, 0, false}
};
-static const struct config_enum_entry server_message_level_options[] = {
+const struct config_enum_entry server_message_level_options[] = {
{"debug", DEBUG2, true},
{"debug5", DEBUG5, false},
{"debug4", DEBUG4, false},
@@ -465,6 +465,7 @@ static char *server_version_string;
static int server_version_num;
static char *timezone_string;
static char *log_timezone_string;
+static char *log_sqlstate_error_statement_str;
static char *timezone_abbreviations_string;
static char *XactIsoLevel_string;
static char *session_authorization_string;
@@ -2947,6 +2948,17 @@ static struct config_string ConfigureNamesString[] =
},
{
+ {"log_sqlstate_error_statement", PGC_SUSET, LOGGING_WHEN,
+ gettext_noop("Overrides minimum error level per error
type"),
+ gettext_noop("Value must be a comma-separated list in
the format "
+ "\"sqlstate:level,...\"."),
+ },
+ &log_sqlstate_error_statement_str,
+ "",
+ check_log_sqlstate_error, assign_log_sqlstate_error, NULL
+ },
+
+ {
{"syslog_ident", PGC_SIGHUP, LOGGING_WHERE,
gettext_noop("Sets the program name used to identify
PostgreSQL "
"messages in syslog."),
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 3adcc99..2ed5677 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -391,4 +391,8 @@ extern bool check_effective_cache_size(int *newval, void
**extra, GucSource sour
extern void set_default_effective_cache_size(void);
extern void assign_xlog_sync_method(int new_sync_method, void *extra);
+/* in src/backend/utils/error/elog.c */
+extern void assign_log_sqlstate_error(const char *newval, void *extra);
+extern bool check_log_sqlstate_error(char **newval, void **extra, GucSource
source);
+
#endif /* GUC_H */
diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h
index 47ff880..1e577a4 100644
--- a/src/include/utils/guc_tables.h
+++ b/src/include/utils/guc_tables.h
@@ -251,6 +251,7 @@ extern const char *const config_group_names[];
extern const char *const config_type_names[];
extern const char *const GucContext_Names[];
extern const char *const GucSource_Names[];
+extern const struct config_enum_entry server_message_level_options[];
/* get the current set of variables */
extern struct config_generic **get_guc_variables(void);
diff --git a/src/test/regress/expected/guc.out
b/src/test/regress/expected/guc.out
index 4f0065c..0d0c4e0 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -736,3 +736,27 @@ NOTICE: text search configuration "no_such_config" does
not exist
select func_with_bad_set();
ERROR: invalid value for parameter "default_text_search_config":
"no_such_config"
reset check_function_bodies;
+-- Test logging options
+SET log_sqlstate_error_statement = 'XX000:PANIC,22012:error,xx000:log';
+SHOW log_sqlstate_error_statement;
+ log_sqlstate_error_statement
+------------------------------
+ XX000:log,22012:error
+(1 row)
+
+SET log_sqlstate_error_statement = '';
+SHOW log_sqlstate_error_statement;
+ log_sqlstate_error_statement
+------------------------------
+
+(1 row)
+
+SET log_sqlstate_error_statement = 'x';
+ERROR: invalid value for parameter "log_sqlstate_error_statement": "x"
+DETAIL: Invalid sqlstate error definition: "x".
+SHOW log_sqlstate_error_statement;
+ log_sqlstate_error_statement
+------------------------------
+
+(1 row)
+
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index 3de8a6b..508a8a4 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -275,3 +275,11 @@ set default_text_search_config = no_such_config;
select func_with_bad_set();
reset check_function_bodies;
+
+-- Test logging options
+SET log_sqlstate_error_statement = 'XX000:PANIC,22012:error,xx000:log';
+SHOW log_sqlstate_error_statement;
+SET log_sqlstate_error_statement = '';
+SHOW log_sqlstate_error_statement;
+SET log_sqlstate_error_statement = 'x';
+SHOW log_sqlstate_error_statement;
--
1.8.4.2
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
