On Thu, Jun 12, 2014 at 10:55 PM, Robert Haas <robertmh...@gmail.com> wrote:
> On Wed, Jun 11, 2014 at 7:42 AM, Magnus Hagander <mag...@hagander.net> wrote:
>>> Replication commands like IDENTIFY_COMMAND are not logged even when
>>> log_statements is set to all. Some users who use log_statements to
>>> audit *all* statements might dislike this current situation. So I'm
>>> thinking to change log_statements or add something like log_replication
>>> so that we can log replication commands. Thought?
>>
>> +1. I think adding a separate parameter is the way to go.
>>
>> The other option would be to turn log_statements into a parameter that you
>> specify multiple ones
>
> I kind of like this idea, but...
>
>> - so instead of "all" today it would be "ddl,dml,all"
>> or something like that, and then you'd also add "replication" as an option.
>> But that would cause all sorts of backwards compatibility annoyances.. And
>> do you really want to be able to say things like "ddl,all" meanin you'd get
>> ddl and select but not dml?
>
> ...you lost me here. I mean, I think it could be quite useful to
> redefine the existing GUC as a list. We could continue to have ddl,
> dml, and all as tokens that would be in the list, but you wouldn't
> write "ddl,dml,all" because "all" would include everything that those
> other ones would log. But then you could have combinations like
> "dml,replication" and so on.
Yep, that seems useful, even aside from logging of replication command topic.
OK, I've just implemented the patch (attached) which does this, i.e., redefines
log_statement as a list. Thanks to the patch, log_statement can be set
to "none",
"ddl", "mod", "dml", "all", and any combinations of them. The meanings of
"none", "ddl", "mod" and "all" are the same as they are now. New setting value
"dml" loggs both data modification statements like INSERT and query access
statements like SELECT and COPY TO.
What about applying this patch first?
Regards,
--
Fujii Masao
From e6a67acd0866e2b14fc716ef8a17cc7ac870e50f Mon Sep 17 00:00:00 2001
From: MasaoFujii <masao.fu...@gmail.com>
Date: Fri, 20 Jun 2014 20:39:08 +0900
Subject: [PATCH] Redefine log_statement as a list.
---
doc/src/sgml/config.sgml | 14 +++++-
src/backend/tcop/fastpath.c | 2 +-
src/backend/tcop/postgres.c | 3 +-
src/backend/tcop/utility.c | 60 +++++++++++++-------------
src/backend/utils/misc/guc.c | 97 ++++++++++++++++++++++++++++++++++--------
src/include/tcop/tcopprot.h | 15 +++---
src/include/tcop/utility.h | 2 +-
7 files changed, 132 insertions(+), 61 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 8f0ca4c..e93dd37 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -4490,7 +4490,7 @@ FROM pg_stat_activity;
</varlistentry>
<varlistentry id="guc-log-statement" xreflabel="log_statement">
- <term><varname>log_statement</varname> (<type>enum</type>)
+ <term><varname>log_statement</varname> (<type>string</type>)
<indexterm>
<primary><varname>log_statement</> configuration parameter</primary>
</indexterm>
@@ -4498,14 +4498,16 @@ FROM pg_stat_activity;
<listitem>
<para>
Controls which SQL statements are logged. Valid values are
- <literal>none</> (off), <literal>ddl</>, <literal>mod</>, and
- <literal>all</> (all statements). <literal>ddl</> logs all data definition
+ <literal>none</> (off), <literal>ddl</>, <literal>mod</>, <literal>dml</>,
+ and <literal>all</> (all statements). <literal>ddl</> logs all data definition
statements, such as <command>CREATE</>, <command>ALTER</>, and
<command>DROP</> statements. <literal>mod</> logs all
<literal>ddl</> statements, plus data-modifying statements
such as <command>INSERT</>,
<command>UPDATE</>, <command>DELETE</>, <command>TRUNCATE</>,
and <command>COPY FROM</>.
+ <literal>dml</> logs all data-modifying statements, plus query access
+ statements such as <command>SELECT</> and <command>COPY TO</>.
<command>PREPARE</>, <command>EXECUTE</>, and
<command>EXPLAIN ANALYZE</> statements are also logged if their
contained command is of an appropriate type. For clients using
@@ -4515,6 +4517,12 @@ FROM pg_stat_activity;
</para>
<para>
+ This parameter can be set to a list of desired SQL statements separated by
+ commas. Note that if <literal>none</> is specified in the list, other settings
+ are ignored and then no SQL statements are logged.
+ </para>
+
+ <para>
The default is <literal>none</>. Only superusers can change this
setting.
</para>
diff --git a/src/backend/tcop/fastpath.c b/src/backend/tcop/fastpath.c
index 9f50c5a..c170e54 100644
--- a/src/backend/tcop/fastpath.c
+++ b/src/backend/tcop/fastpath.c
@@ -340,7 +340,7 @@ HandleFunctionRequest(StringInfo msgBuf)
fetch_fp_info(fid, fip);
/* Log as soon as we have the function OID and name */
- if (log_statement == LOGSTMT_ALL)
+ if (log_statement & LOGSTMT_OTHER)
{
ereport(LOG,
(errmsg("fastpath function call: \"%s\" (OID %u)",
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 6b4221a..8a78989 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -89,6 +89,7 @@ CommandDest whereToSendOutput = DestDebug;
bool Log_disconnections = false;
int log_statement = LOGSTMT_NONE;
+char *log_statement_string = NULL;
/* GUC variable for maximum stack depth (measured in kilobytes) */
int max_stack_depth = 100;
@@ -2017,7 +2018,7 @@ check_log_statement(List *stmt_list)
{
Node *stmt = (Node *) lfirst(stmt_item);
- if (GetCommandLogLevel(stmt) <= log_statement)
+ if (GetCommandLogLevel(stmt) & log_statement)
return true;
}
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 3423898..8498e30 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -2448,10 +2448,10 @@ CreateCommandTag(Node *parsetree)
* This must handle all command types, but since the vast majority
* of 'em are utility commands, it seems sensible to keep it here.
*/
-LogStmtLevel
+int
GetCommandLogLevel(Node *parsetree)
{
- LogStmtLevel lev;
+ int lev;
switch (nodeTag(parsetree))
{
@@ -2466,24 +2466,24 @@ GetCommandLogLevel(Node *parsetree)
if (((SelectStmt *) parsetree)->intoClause)
lev = LOGSTMT_DDL; /* SELECT INTO */
else
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_QUERY;
break;
/* utility statements --- same whether raw or cooked */
case T_TransactionStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_DeclareCursorStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_ClosePortalStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_FetchStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER; /* should this be QUERY? */
break;
case T_CreateSchemaStmt:
@@ -2541,7 +2541,7 @@ GetCommandLogLevel(Node *parsetree)
if (((CopyStmt *) parsetree)->is_from)
lev = LOGSTMT_MOD;
else
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_QUERY;
break;
case T_PrepareStmt:
@@ -2563,12 +2563,12 @@ GetCommandLogLevel(Node *parsetree)
if (ps)
lev = GetCommandLogLevel(ps->plansource->raw_parse_tree);
else
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
}
break;
case T_DeallocateStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_RenameStmt:
@@ -2652,7 +2652,7 @@ GetCommandLogLevel(Node *parsetree)
break;
case T_DoStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_CreatedbStmt:
@@ -2672,19 +2672,19 @@ GetCommandLogLevel(Node *parsetree)
break;
case T_NotifyStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_ListenStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_UnlistenStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_LoadStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_ClusterStmt:
@@ -2692,7 +2692,7 @@ GetCommandLogLevel(Node *parsetree)
break;
case T_VacuumStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_ExplainStmt:
@@ -2714,7 +2714,7 @@ GetCommandLogLevel(Node *parsetree)
return GetCommandLogLevel(stmt->query);
/* Plain EXPLAIN isn't so interesting */
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
}
break;
@@ -2727,19 +2727,19 @@ GetCommandLogLevel(Node *parsetree)
break;
case T_AlterSystemStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_VariableSetStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_VariableShowStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_DiscardStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_CreateTrigStmt:
@@ -2787,19 +2787,19 @@ GetCommandLogLevel(Node *parsetree)
break;
case T_LockStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_ConstraintsSetStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_CheckPointStmt:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
case T_ReindexStmt:
- lev = LOGSTMT_ALL; /* should this be DDL? */
+ lev = LOGSTMT_OTHER; /* should this be DDL? */
break;
case T_CreateConversionStmt:
@@ -2838,7 +2838,7 @@ GetCommandLogLevel(Node *parsetree)
switch (stmt->commandType)
{
case CMD_SELECT:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_QUERY;
break;
case CMD_UPDATE:
@@ -2850,7 +2850,7 @@ GetCommandLogLevel(Node *parsetree)
default:
elog(WARNING, "unrecognized commandType: %d",
(int) stmt->commandType);
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
}
}
@@ -2864,7 +2864,7 @@ GetCommandLogLevel(Node *parsetree)
switch (stmt->commandType)
{
case CMD_SELECT:
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_QUERY;
break;
case CMD_UPDATE:
@@ -2880,7 +2880,7 @@ GetCommandLogLevel(Node *parsetree)
default:
elog(WARNING, "unrecognized commandType: %d",
(int) stmt->commandType);
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
}
@@ -2890,7 +2890,7 @@ GetCommandLogLevel(Node *parsetree)
default:
elog(WARNING, "unrecognized node type: %d",
(int) nodeTag(parsetree));
- lev = LOGSTMT_ALL;
+ lev = LOGSTMT_OTHER;
break;
}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 6902c23..67c3552 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -160,6 +160,8 @@ static bool call_string_check_hook(struct config_string * conf, char **newval,
static bool call_enum_check_hook(struct config_enum * conf, int *newval,
void **extra, GucSource source, int elevel);
+static bool check_logstmt(char **newval, void **extra, GucSource source);
+static void assign_logstmt(const char *newval, void *extra);
static bool check_log_destination(char **newval, void **extra, GucSource source);
static void assign_log_destination(const char *newval, void *extra);
@@ -275,14 +277,6 @@ static const struct config_enum_entry log_error_verbosity_options[] = {
{NULL, 0, false}
};
-static const struct config_enum_entry log_statement_options[] = {
- {"none", LOGSTMT_NONE, false},
- {"ddl", LOGSTMT_DDL, false},
- {"mod", LOGSTMT_MOD, false},
- {"all", LOGSTMT_ALL, false},
- {NULL, 0, false}
-};
-
static const struct config_enum_entry isolation_level_options[] = {
{"serializable", XACT_SERIALIZABLE, false},
{"repeatable read", XACT_REPEATABLE_READ, false},
@@ -2966,6 +2960,17 @@ static struct config_string ConfigureNamesString[] =
},
{
+ {"log_statement", PGC_SUSET, LOGGING_WHAT,
+ gettext_noop("Sets the type of statements logged."),
+ NULL,
+ GUC_LIST_INPUT
+ },
+ &log_statement_string,
+ "none",
+ check_logstmt, assign_logstmt, NULL
+ },
+
+ {
{"log_destination", PGC_SIGHUP, LOGGING_WHERE,
gettext_noop("Sets the destination for server log output."),
gettext_noop("Valid values are combinations of \"stderr\", "
@@ -3366,16 +3371,6 @@ static struct config_enum ConfigureNamesEnum[] =
},
{
- {"log_statement", PGC_SUSET, LOGGING_WHAT,
- gettext_noop("Sets the type of statements logged."),
- NULL
- },
- &log_statement,
- LOGSTMT_NONE, log_statement_options,
- NULL, NULL, NULL
- },
-
- {
{"syslog_facility", PGC_SIGHUP, LOGGING_WHERE,
gettext_noop("Sets the syslog \"facility\" to be used when syslog enabled."),
NULL
@@ -8993,6 +8988,72 @@ call_enum_check_hook(struct config_enum * conf, int *newval, void **extra,
*/
static bool
+check_logstmt(char **newval, void **extra, GucSource source)
+{
+ char *rawstring;
+ List *elemlist;
+ ListCell *l;
+ int newlogstmt = 0;
+ int *myextra;
+ bool nolog = false;
+
+ /* 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;
+ }
+
+ foreach(l, elemlist)
+ {
+ char *tok = (char *) lfirst(l);
+
+ if (pg_strcasecmp(tok, "none") == 0)
+ nolog = true;
+ else if (pg_strcasecmp(tok, "ddl") == 0)
+ newlogstmt |= LOGSTMT_DDL;
+ else if (pg_strcasecmp(tok, "mod") == 0)
+ newlogstmt |= (LOGSTMT_DDL | LOGSTMT_MOD);
+ else if (pg_strcasecmp(tok, "dml") == 0)
+ newlogstmt |= (LOGSTMT_MOD | LOGSTMT_QUERY);
+ else if (pg_strcasecmp(tok, "all") == 0)
+ newlogstmt = LOGSTMT_ALL;
+ else
+ {
+ GUC_check_errdetail("Unrecognized key word: \"%s\".", tok);
+ pfree(rawstring);
+ list_free(elemlist);
+ return false;
+ }
+ }
+
+ /* If "none" is set, ignore other settings and don't log any statements */
+ if (nolog)
+ newlogstmt = LOGSTMT_NONE;
+
+ pfree(rawstring);
+ list_free(elemlist);
+
+ myextra = (int *) guc_malloc(ERROR, sizeof(int));
+ *myextra = newlogstmt;
+ *extra = (void *) myextra;
+
+ return true;
+}
+
+static void
+assign_logstmt(const char *newval, void *extra)
+{
+ log_statement = *((int *) extra);
+}
+
+static bool
check_log_destination(char **newval, void **extra, GucSource source)
{
char *rawstring;
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h
index 60f7532..837111c 100644
--- a/src/include/tcop/tcopprot.h
+++ b/src/include/tcop/tcopprot.h
@@ -36,15 +36,16 @@ extern int PostAuthDelay;
/* GUC-configurable parameters */
-typedef enum
-{
- LOGSTMT_NONE, /* log no statements */
- LOGSTMT_DDL, /* log data definition statements */
- LOGSTMT_MOD, /* log modification statements, plus DDL */
- LOGSTMT_ALL /* log all statements */
-} LogStmtLevel;
+/* Log statement bigmap */
+#define LOGSTMT_NONE 0 /* log no statements */
+#define LOGSTMT_DDL 1 /* log data definition statements */
+#define LOGSTMT_MOD 2 /* log modification statements */
+#define LOGSTMT_QUERY 4 /* log query access statements */
+#define LOGSTMT_OTHER 8 /* log other statements */
+#define LOGSTMT_ALL 15 /* log all statements */
extern int log_statement;
+extern char *log_statement_string;
extern List *pg_parse_query(const char *query_string);
extern List *pg_analyze_and_rewrite(Node *parsetree, const char *query_string,
diff --git a/src/include/tcop/utility.h b/src/include/tcop/utility.h
index 3ecf931..a3852da 100644
--- a/src/include/tcop/utility.h
+++ b/src/include/tcop/utility.h
@@ -45,7 +45,7 @@ extern Query *UtilityContainsQuery(Node *parsetree);
extern const char *CreateCommandTag(Node *parsetree);
-extern LogStmtLevel GetCommandLogLevel(Node *parsetree);
+extern int GetCommandLogLevel(Node *parsetree);
extern bool CommandIsReadOnly(Node *parsetree);
--
1.7.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
