Hi,
I'm sorry for replying so late.
I don't think those really are contradictions. You can continue to have
an errdetail_params(), and but call it from the error context callback
set up in the portal code
...
Even leaving that aside, I'm *STRONGLY* against entangling elog.c with
query execution details like ParamListInfo. We should work to make
elog.c know less about different parts of the system, not more.
I've implemented it using error contexts, please could you have a look
at the patch attached?
I did not use the PG_TRY blocks in portal code because they are not wide
enough.
Otherwise, we wouldn't catch errors that can happen in GetCachedPlan.
Instead I added an item to error context chain in postgres.c
exec_*_message routines.
I can't find why it could be unsafe, but I'm still a bit worried about
it as there's no
other error context changes that early in the call stack.
I've also made string formatting more efficient, and pre-calculate
the whole string instead of individual values as suggested. Unfortunately
it still copies the error text when reporting -- I'm not sure how I
change it
without substantial change of error logging infrastructure.
One more concern I'd like to ask for opinions. I'm using errdetail_log
for parameters logging, as it's the only way to make it appear in the
log only
but not get sent to the client. It looks a bit awkward, as it appears like
ERROR
DETAIL
CONTEXT
STATEMENT,
where CONTEXT may in fact be something inner nested than the parameters
that appear in DETAILS.
With this regards, I'm thinking of adding some special arrangements into
elog.c.
One idea is a econtext_log to add a log-only data into CONTEXT, however
it'll still log
params above the statement, although directly above.
Another option is a special estatementparams (or perhaps
efinalcontext_log?) to log
below the statement.
What do you think?
2 Alvaro:
So, if some parameters are large (they can be up to 1 GB-1, remember)
then we can bloat the log file severely. I think we need to place an
upper limit on the strings that we're going to log -- as inspiration,
callers of ExecBuildValueDescription uses 64 chars per value maximum.
Something like that seems reasonable. So I think you need to add some
pg_mbcliplen() calls in a couple of places in exec_bind_message.
I like the idea, but I don't think it's directly related to the change
proposed. We are already logging parameters in certain situations
with no limits on their lenghts. Given the time it takes to get the change
through (not least because of me addressing reviewers' comments
really slowly) I'd not include it in the patch. Do you find it acceptable?
Best, Alex
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 4ec13f3311..b3a0d27861 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6597,6 +6597,29 @@ log_line_prefix = '%m [%p] %q%u@%d/%a '
</listitem>
</varlistentry>
+ <varlistentry id="guc-log-parameters-on-error" xreflabel="log_parameters_on_error">
+ <term><varname>log_parameters_on_error</varname> (<type>boolean</type>)
+ <indexterm>
+ <primary><varname>log_parameters_on_error</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Controls whether bind parameters are logged when a statement is logged
+ as a result of <xref linkend="guc-log-min-error-statement"/>.
+ It adds some overhead, as postgres will compute and store textual
+ representations of parameter values in memory for all statements,
+ even if they eventually do not get logged.
+ This setting has no effect on statements logged due to
+ <xref linkend="guc-log-min-duration-statement"/> or
+ <xref linkend="guc-log-statement"/> settings, as they are always logged
+ with parameters.
+ The default is <literal>off</literal>.
+ Only superusers can change this setting.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-log-statement" xreflabel="log_statement">
<term><varname>log_statement</varname> (<type>enum</type>)
<indexterm>
diff --git a/src/backend/nodes/params.c b/src/backend/nodes/params.c
index cf4387e40f..7c1f86f5c5 100644
--- a/src/backend/nodes/params.c
+++ b/src/backend/nodes/params.c
@@ -15,12 +15,16 @@
#include "postgres.h"
+#include "lib/stringinfo.h"
#include "nodes/bitmapset.h"
#include "nodes/params.h"
#include "storage/shmem.h"
#include "utils/datum.h"
+#include "utils/guc.h"
#include "utils/lsyscache.h"
+#include "utils/memutils.h"
+static void LogParams(void *arg);
/*
* Allocate and initialize a new ParamListInfo structure.
@@ -44,6 +48,7 @@ makeParamList(int numParams)
retval->paramCompileArg = NULL;
retval->parserSetup = NULL;
retval->parserSetupArg = NULL;
+ retval->logString = NULL;
retval->numParams = numParams;
return retval;
@@ -58,6 +63,8 @@ makeParamList(int numParams)
* set of parameter values. If dynamic parameter hooks are present, we
* intentionally do not copy them into the result. Rather, we forcibly
* instantiate all available parameter values and copy the datum values.
+ *
+ * We also don't copy logString.
*/
ParamListInfo
copyParamList(ParamListInfo from)
@@ -221,6 +228,8 @@ SerializeParamList(ParamListInfo paramLI, char **start_address)
* set of parameter values. If dynamic parameter hooks are present, we
* intentionally do not copy them into the result. Rather, we forcibly
* instantiate all available parameter values and copy the datum values.
+ *
+ * We also don't copy logString.
*/
ParamListInfo
RestoreParamList(char **start_address)
@@ -251,3 +260,152 @@ RestoreParamList(char **start_address)
return paramLI;
}
+
+/*
+ * BuildParamLogString - create a string to represent parameters list for
+ * logging and save it to params. If caller already knows textual
+ * representations for some or all parameters, it can pass an array of exactly
+ * params->numParams values as knownTextValues. It can contain NULLs for the
+ * individual values not known, or, if no text text values known at all the
+ * caller may pass NULL pointer.
+ */
+void
+BuildParamLogString(ParamListInfo params, char **knownTextValues)
+{
+ MemoryContext tmpCxt,
+ oldCxt;
+ int logStringLen = 0;
+ StringInfoData logString;
+ char **textValues;
+
+ if (params->logString != NULL)
+ return;
+
+ Assert(params->paramFetch == NULL);
+
+ initStringInfo(&logString);
+
+ tmpCxt = AllocSetContextCreate(CurrentMemoryContext,
+ "BuildParamLogString",
+ ALLOCSET_DEFAULT_SIZES);
+ oldCxt = MemoryContextSwitchTo(tmpCxt);
+
+ textValues = (char **) palloc0(params->numParams * sizeof(char *));
+
+ /* calculate unknown text representations and pre-calculate byte lengths */
+ for (int paramno = 0; paramno < params->numParams; paramno++)
+ {
+ ParamExternData param = params->params[paramno];
+
+ /* reserve the space for the number of digits in paramno + 1 */
+ for (int d = paramno + 1; d > 0; d /= 10)
+ logStringLen++;
+
+ if (param.isnull)
+ {
+ logStringLen += 4; /* for "NULL" */
+ }
+ else
+ {
+ char *textValue,
+ *s;
+
+ /* find out textValues[paramno] */
+ if (knownTextValues != NULL && knownTextValues[paramno] != NULL)
+ {
+ textValues[paramno] = knownTextValues[paramno];
+ }
+ else
+ {
+ Oid typoutput;
+ bool typisvarlena;
+ getTypeOutputInfo(param.ptype, &typoutput, &typisvarlena);
+ textValues[paramno] =
+ OidOutputFunctionCall(typoutput, param.value);
+ }
+
+ /* caluclate the space needed its printed length */
+ textValue = textValues[paramno];
+ for (s = textValue; *s != '\0'; s++)
+ if (*s == '\'')
+ logStringLen++; /* for quotes doubling */
+ /* string length + 2 for quotes */
+ logStringLen += s - textValue + 2;
+ }
+ }
+
+ /*
+ * "$1 = something, " -- 6 bytes extra to the ordinal and the value;
+ * ^ ^^^ ^^
+ * for the last one we don't need the comma and the space but need 0 byte
+ */
+ logStringLen += params->numParams * 6 - 1;
+
+ MemoryContextSwitchTo(oldCxt);
+
+ /* enlarge at once to avoid multiple reallocations */
+ enlargeStringInfo(&logString, logStringLen);
+
+ for (int paramno = 0; paramno < params->numParams; paramno++)
+ {
+ appendStringInfo(&logString,
+ "%s$%d = ",
+ paramno > 0 ? ", " : "",
+ paramno + 1);
+ appendStringInfoStringQuoted(&logString, textValues[paramno]);
+ }
+
+ params->logString = logString.data;
+
+ MemoryContextDelete(tmpCxt);
+}
+
+/*
+ * PutParamsInErrorContext - if needed and possible,
+ * add an error context entry with bind parameters
+ *
+ * params is a pointer to ParamListInfo for the caller to be able to clear it
+ * if the actual ParamListInfo is gone
+ */
+void
+PutParamsInErrorContext(ErrorContextCallback *new_error_context_entry,
+ ParamListInfo *params)
+{
+ /*
+ * Setting this anyway for the caller to be able to revert this operation
+ * with `error_context_stack = new_error_context_entry->previous` regardless
+ * whether we actually change the stack
+ */
+ new_error_context_entry->previous = error_context_stack;
+
+ if (!log_parameters_on_error || *params == NULL
+ || (*params)->logString == NULL)
+ return;
+
+ new_error_context_entry->callback = LogParams;
+ new_error_context_entry->arg = (void *)(params);
+ error_context_stack = new_error_context_entry;
+}
+
+/*
+ * LogParams - callback for printing parameters in error context
+ */
+static void
+LogParams(void *arg)
+{
+ ParamListInfo params = *((ParamListInfo *)(arg));
+
+ /*
+ * XXX: errdetail_log copies the error message formatting, so we're
+ * allocating a potentially large amounts of memory for the whole parameter
+ * string again
+ *
+ * We can have NULL in params->logString if log_parameters_on_error was off
+ * when we decided whether to call BuildParamLogString. In this rare case
+ * we don't log the parameters: building the log string now wouldn't be safe
+ * as we must't call user-defined I/O functions when in an aborted xact.
+ */
+ if (params != NULL && params->logString != NULL
+ && log_parameters_on_error && geterrshowstmt())
+ errdetail_log("parameters: %s", params->logString);
+}
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 3b85e48333..9b562f7f01 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -177,7 +177,7 @@ static void forbidden_in_wal_sender(char firstchar);
static List *pg_rewrite_query(Query *query);
static bool check_log_statement(List *stmt_list);
static int errdetail_execute(List *raw_parsetree_list);
-static int errdetail_params(ParamListInfo params);
+static int errdetail_portal_params(Portal portal);
static int errdetail_abort(void);
static int errdetail_recovery_conflict(void);
static void start_xact_command(void);
@@ -1596,23 +1596,24 @@ exec_parse_message(const char *query_string, /* string to execute */
static void
exec_bind_message(StringInfo input_message)
{
- const char *portal_name;
- const char *stmt_name;
- int numPFormats;
- int16 *pformats = NULL;
- int numParams;
- int numRFormats;
- int16 *rformats = NULL;
- CachedPlanSource *psrc;
- CachedPlan *cplan;
- Portal portal;
- char *query_string;
- char *saved_stmt_name;
- ParamListInfo params;
- MemoryContext oldContext;
- bool save_log_statement_stats = log_statement_stats;
- bool snapshot_set = false;
- char msec_str[32];
+ const char *portal_name;
+ const char *stmt_name;
+ int numPFormats;
+ int16 *pformats = NULL;
+ int numParams;
+ int numRFormats;
+ int16 *rformats = NULL;
+ CachedPlanSource *psrc;
+ CachedPlan *cplan;
+ Portal portal;
+ char *query_string;
+ char *saved_stmt_name;
+ ParamListInfo params;
+ MemoryContext oldContext;
+ bool save_log_statement_stats = log_statement_stats;
+ bool snapshot_set = false;
+ char msec_str[32];
+ ErrorContextCallback params_error_context;
/* Get the fixed part of the message */
portal_name = pq_getmsgstring(input_message);
@@ -1752,6 +1753,10 @@ exec_bind_message(StringInfo input_message)
*/
if (numParams > 0)
{
+ /* These are short-living, so we don't pollute the portal context */
+ char **knownTextValues = MemoryContextAllocZero(MessageContext,
+ numParams * sizeof(char *));
+
params = makeParamList(numParams);
for (int paramno = 0; paramno < numParams; paramno++)
@@ -1819,9 +1824,17 @@ exec_bind_message(StringInfo input_message)
pval = OidInputFunctionCall(typinput, pstring, typioparam, -1);
- /* Free result of encoding conversion, if any */
- if (pstring && pstring != pbuf.data)
- pfree(pstring);
+ /*
+ * If we need the parameter string, save it.
+ * If not -- get rid of it.
+ */
+ if (pstring)
+ {
+ if (log_parameters_on_error)
+ knownTextValues[paramno] = pstring;
+ else if (pstring != pbuf.data)
+ pfree(pstring);
+ }
}
else if (pformat == 1) /* binary mode */
{
@@ -1871,6 +1884,9 @@ exec_bind_message(StringInfo input_message)
params->params[paramno].pflags = PARAM_FLAG_CONST;
params->params[paramno].ptype = ptype;
}
+
+ if (log_parameters_on_error)
+ BuildParamLogString(params, knownTextValues);
}
else
params = NULL;
@@ -1878,6 +1894,9 @@ exec_bind_message(StringInfo input_message)
/* Done storing stuff in portal's context */
MemoryContextSwitchTo(oldContext);
+ /* From now on, log parameters on error if enabled in GUC */
+ PutParamsInErrorContext(¶ms_error_context, ¶ms);
+
/* Get the result format codes */
numRFormats = pq_getmsgint(input_message, 2);
if (numRFormats > 0)
@@ -1948,13 +1967,14 @@ exec_bind_message(StringInfo input_message)
*portal_name ? portal_name : "",
psrc->query_string),
errhidestmt(true),
- errdetail_params(params)));
+ errdetail_portal_params(portal)));
break;
}
if (save_log_statement_stats)
ShowUsage("BIND MESSAGE STATISTICS");
+ error_context_stack = params_error_context.previous;
debug_query_string = NULL;
}
@@ -1966,19 +1986,20 @@ exec_bind_message(StringInfo input_message)
static void
exec_execute_message(const char *portal_name, long max_rows)
{
- CommandDest dest;
- DestReceiver *receiver;
- Portal portal;
- bool completed;
- char completionTag[COMPLETION_TAG_BUFSIZE];
- const char *sourceText;
- const char *prepStmtName;
- ParamListInfo portalParams;
- bool save_log_statement_stats = log_statement_stats;
- bool is_xact_command;
- bool execute_is_fetch;
- bool was_logged = false;
- char msec_str[32];
+ CommandDest dest;
+ DestReceiver *receiver;
+ Portal portal;
+ ParamListInfo params;
+ bool completed;
+ char completionTag[COMPLETION_TAG_BUFSIZE];
+ const char *sourceText;
+ const char *prepStmtName;
+ bool save_log_statement_stats = log_statement_stats;
+ bool is_xact_command;
+ bool execute_is_fetch;
+ bool was_logged = false;
+ char msec_str[32];
+ ErrorContextCallback params_error_context;
/* Adjust destination to tell printtup.c what to do */
dest = whereToSendOutput;
@@ -2002,6 +2023,10 @@ exec_execute_message(const char *portal_name, long max_rows)
return;
}
+ /* From now on, log parameters on error if enabled in GUC */
+ params = portal->portalParams;
+ PutParamsInErrorContext(¶ms_error_context, ¶ms);
+
/* Does the portal contain a transaction command? */
is_xact_command = IsTransactionStmtList(portal->stmts);
@@ -2017,12 +2042,6 @@ exec_execute_message(const char *portal_name, long max_rows)
prepStmtName = pstrdup(portal->prepStmtName);
else
prepStmtName = "<unnamed>";
-
- /*
- * An xact command shouldn't have any parameters, which is a good
- * thing because they wouldn't be around after finish_xact_command.
- */
- portalParams = NULL;
}
else
{
@@ -2031,7 +2050,6 @@ exec_execute_message(const char *portal_name, long max_rows)
prepStmtName = portal->prepStmtName;
else
prepStmtName = "<unnamed>";
- portalParams = portal->portalParams;
}
/*
@@ -2083,7 +2101,7 @@ exec_execute_message(const char *portal_name, long max_rows)
*portal_name ? portal_name : "",
sourceText),
errhidestmt(true),
- errdetail_params(portalParams)));
+ errdetail_portal_params(portal)));
was_logged = true;
}
@@ -2122,6 +2140,15 @@ exec_execute_message(const char *portal_name, long max_rows)
{
if (is_xact_command)
{
+ /*
+ * The portal and its params may be destroyed during
+ * finish_xact_command below. We only may need the portal for
+ * parameters logging, but an xact command shouldn't have any
+ * parameters.
+ */
+ portal = NULL;
+ params = NULL;
+
/*
* If this was a transaction control statement, commit it. We
* will start a new xact command for the next command (if any).
@@ -2175,13 +2202,14 @@ exec_execute_message(const char *portal_name, long max_rows)
*portal_name ? portal_name : "",
sourceText),
errhidestmt(true),
- errdetail_params(portalParams)));
+ errdetail_portal_params(portal)));
break;
}
if (save_log_statement_stats)
ShowUsage("EXECUTE MESSAGE STATISTICS");
+ error_context_stack = params_error_context.previous;
debug_query_string = NULL;
}
@@ -2321,66 +2349,26 @@ errdetail_execute(List *raw_parsetree_list)
}
/*
- * errdetail_params
+ * errdetail_portal_params
*
* Add an errdetail() line showing bind-parameter data, if available.
*/
static int
-errdetail_params(ParamListInfo params)
+errdetail_portal_params(Portal portal)
{
- /* We mustn't call user-defined I/O functions when in an aborted xact */
- if (params && params->numParams > 0 && !IsAbortedTransactionBlockState())
+ if (portal != NULL &&
+ portal->portalParams &&
+ portal->portalParams->numParams > 0)
{
- StringInfoData param_str;
- MemoryContext oldcontext;
-
- /* This code doesn't support dynamic param lists */
- Assert(params->paramFetch == NULL);
-
- /* Make sure any trash is generated in MessageContext */
- oldcontext = MemoryContextSwitchTo(MessageContext);
-
- initStringInfo(¶m_str);
-
- for (int paramno = 0; paramno < params->numParams; paramno++)
- {
- ParamExternData *prm = ¶ms->params[paramno];
- Oid typoutput;
- bool typisvarlena;
- char *pstring;
- char *p;
-
- appendStringInfo(¶m_str, "%s$%d = ",
- paramno > 0 ? ", " : "",
- paramno + 1);
-
- if (prm->isnull || !OidIsValid(prm->ptype))
- {
- appendStringInfoString(¶m_str, "NULL");
- continue;
- }
-
- getTypeOutputInfo(prm->ptype, &typoutput, &typisvarlena);
-
- pstring = OidOutputFunctionCall(typoutput, prm->value);
-
- appendStringInfoCharMacro(¶m_str, '\'');
- for (p = pstring; *p; p++)
- {
- if (*p == '\'') /* double single quotes */
- appendStringInfoCharMacro(¶m_str, *p);
- appendStringInfoCharMacro(¶m_str, *p);
- }
- appendStringInfoCharMacro(¶m_str, '\'');
-
- pfree(pstring);
- }
-
- errdetail("parameters: %s", param_str.data);
-
- pfree(param_str.data);
+ /*
+ * We switch back to portal context for the params logString to be
+ * generated there in order to reuse it in later invocations
+ */
+ MemoryContext old_cxt = MemoryContextSwitchTo(portal->portalContext);
+ BuildParamLogString(portal->portalParams, NULL);
+ MemoryContextSwitchTo(old_cxt);
- MemoryContextSwitchTo(oldcontext);
+ errdetail("parameters: %s", portal->portalParams->logString);
}
return 0;
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index f46653632e..eb39c750f4 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -1363,6 +1363,22 @@ getinternalerrposition(void)
return edata->internalpos;
}
+/*
+ * geterrshowstmt --- return whether we are going to print the statement in the log
+ *
+ * This is only intended for use in error callback subroutines, since there
+ * is no other place outside elog.c where the concept is meaningful.
+ */
+bool
+geterrshowstmt(void)
+{
+ ErrorData *edata = &errordata[errordata_stack_depth];
+
+ return is_log_level_output(edata->elevel, log_min_error_statement) &&
+ debug_query_string != NULL &&
+ !edata->hide_stmt;
+}
+
/*
* elog_start --- startup for old-style API
@@ -2742,7 +2758,7 @@ static void
write_csvlog(ErrorData *edata)
{
StringInfoData buf;
- bool print_stmt = false;
+ bool print_stmt = geterrshowstmt();
/* static counter for line numbers */
static long log_line_number = 0;
@@ -2886,10 +2902,6 @@ 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) &&
- debug_query_string != NULL &&
- !edata->hide_stmt)
- print_stmt = true;
if (print_stmt)
appendCSVLiteral(&buf, debug_query_string);
appendStringInfoChar(&buf, ',');
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 5fccc9683e..1be1f1fb29 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -486,6 +486,7 @@ extern const struct config_enum_entry dynamic_shared_memory_options[];
* GUC option variables that are exported from this module
*/
bool log_duration = false;
+bool log_parameters_on_error = false;
bool Debug_print_plan = false;
bool Debug_print_parse = false;
bool Debug_print_rewritten = false;
@@ -1300,6 +1301,15 @@ static struct config_bool ConfigureNamesBool[] =
false,
NULL, NULL, NULL
},
+ {
+ {"log_parameters_on_error", PGC_SUSET, LOGGING_WHAT,
+ gettext_noop("Logs bind parameters of the logged statements where possible."),
+ NULL
+ },
+ &log_parameters_on_error,
+ false,
+ NULL, NULL, NULL
+ },
{
{"debug_print_parse", PGC_USERSET, LOGGING_WHAT,
gettext_noop("Logs each query's parse tree."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 46a06ffacd..91cdc06fc4 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -544,6 +544,7 @@
# e.g. '<%u%%%d> '
#log_lock_waits = off # log lock waits >= deadlock_timeout
#log_statement = 'none' # none, ddl, mod, all
+#log_parameters_on_error = off # on error log statements with bind parameters
#log_replication_commands = off
#log_temp_files = -1 # log temporary files equal or larger
# than the specified size in kilobytes;
diff --git a/src/common/stringinfo.c b/src/common/stringinfo.c
index a50e587da9..300fa4527b 100644
--- a/src/common/stringinfo.c
+++ b/src/common/stringinfo.c
@@ -178,6 +178,42 @@ appendStringInfoString(StringInfo str, const char *s)
appendBinaryStringInfo(str, s, strlen(s));
}
+/*
+ * appendStringInfoStringQuoted
+ *
+ * Append a null-terminated string to str, adding single quotes around
+ * and doubling all single quotes.
+ */
+void
+appendStringInfoStringQuoted(StringInfo str, const char *s)
+{
+ const char *chunk_search_start = s,
+ *chunk_copy_start = s,
+ *chunk_end;
+
+ Assert(str != NULL);
+
+ appendStringInfoCharMacro(str, '\'');
+
+ while ((chunk_end = strchr(chunk_search_start, '\'')) != NULL)
+ {
+ /* copy including the found delimiting ' */
+ appendBinaryStringInfoNT(str,
+ chunk_copy_start,
+ chunk_end - chunk_copy_start + 1);
+
+ /* in order to double it, include this ' into the next chunk as well*/
+ chunk_copy_start = chunk_end;
+ chunk_search_start = chunk_end + 1;
+ }
+
+ /* copy the last chunk */
+ appendBinaryStringInfoNT(str, chunk_copy_start, strlen(chunk_copy_start));
+
+ appendStringInfoCharMacro(str, '\'');
+ str->data[str->len] = '\0';
+}
+
/*
* appendStringInfoChar
*
diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h
index e27942728e..4978ac9aa3 100644
--- a/src/include/lib/stringinfo.h
+++ b/src/include/lib/stringinfo.h
@@ -113,6 +113,13 @@ extern int appendStringInfoVA(StringInfo str, const char *fmt, va_list args) pg_
*/
extern void appendStringInfoString(StringInfo str, const char *s);
+/*
+ * appendStringInfoStringQuoted
+ * Append a null-terminated string to str, adding single quotes around
+ * and doubling all single quotes.
+ */
+extern void appendStringInfoStringQuoted(StringInfo str, const char *s);
+
/*------------------------
* appendStringInfoChar
* Append a single byte to str.
diff --git a/src/include/nodes/params.h b/src/include/nodes/params.h
index fd9046619c..49a450b6d0 100644
--- a/src/include/nodes/params.h
+++ b/src/include/nodes/params.h
@@ -115,6 +115,7 @@ typedef struct ParamListInfoData
void *paramCompileArg;
ParserSetupHook parserSetup; /* parser setup hook */
void *parserSetupArg;
+ char *logString; /* string to log parameters */
int numParams; /* nominal/maximum # of Params represented */
/*
@@ -156,5 +157,9 @@ extern ParamListInfo copyParamList(ParamListInfo from);
extern Size EstimateParamListSpace(ParamListInfo paramLI);
extern void SerializeParamList(ParamListInfo paramLI, char **start_address);
extern ParamListInfo RestoreParamList(char **start_address);
+extern void BuildParamLogString(ParamListInfo params, char **paramTextValues);
+extern void PutParamsInErrorContext(
+ ErrorContextCallback *new_error_context_entry,
+ ParamListInfo *params);
#endif /* PARAMS_H */
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 9ab4b5470b..ecc78d2ae6 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -202,6 +202,7 @@ extern int err_generic_string(int field, const char *str);
extern int geterrcode(void);
extern int geterrposition(void);
extern int getinternalerrposition(void);
+extern bool geterrshowstmt(void);
/*----------
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 50098e63fe..41d5e1d14a 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -234,6 +234,7 @@ typedef enum
/* GUC vars that are actually declared in guc.c, rather than elsewhere */
extern bool log_duration;
+extern bool log_parameters_on_error;
extern bool Debug_print_plan;
extern bool Debug_print_parse;
extern bool Debug_print_rewritten;
diff --git a/src/test/README b/src/test/README
index b5ccfc0cf6..22e624d8ba 100644
--- a/src/test/README
+++ b/src/test/README
@@ -12,8 +12,7 @@ authentication/
Tests for authentication
examples/
- Demonstration programs for libpq that double as regression tests via
- "make check"
+ Demonstration programs for libpq that double as regression tests
isolation/
Tests for concurrent behavior at the SQL level
diff --git a/src/test/examples/.gitignore b/src/test/examples/.gitignore
index 1957ec198f..007aaee590 100644
--- a/src/test/examples/.gitignore
+++ b/src/test/examples/.gitignore
@@ -2,5 +2,6 @@
/testlibpq2
/testlibpq3
/testlibpq4
+/testlibpq5
/testlo
/testlo64
diff --git a/src/test/examples/Makefile b/src/test/examples/Makefile
index a67f456904..0407ca60bc 100644
--- a/src/test/examples/Makefile
+++ b/src/test/examples/Makefile
@@ -14,7 +14,7 @@ override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
LDFLAGS_INTERNAL += $(libpq_pgport)
-PROGS = testlibpq testlibpq2 testlibpq3 testlibpq4 testlo testlo64
+PROGS = testlibpq testlibpq2 testlibpq3 testlibpq4 testlibpq5 testlo testlo64
all: $(PROGS)
diff --git a/src/test/examples/testlibpq5.c b/src/test/examples/testlibpq5.c
new file mode 100644
index 0000000000..47a5e31335
--- /dev/null
+++ b/src/test/examples/testlibpq5.c
@@ -0,0 +1,193 @@
+/*
+ * src/test/examples/testlibpq5.c
+ *
+ * testlibpq5.c
+ * Test logging of statement parameters in case of errors.
+ */
+
+#ifdef WIN32
+#include <windows.h>
+#endif
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+#include <sys/types.h>
+#include "libpq-fe.h"
+
+static void
+exit_nicely(PGconn *conn)
+{
+ PQfinish(conn);
+ exit(1);
+}
+
+int
+main(int argc, char **argv)
+{
+ const char *conninfo;
+ PGconn *conn;
+ PGresult *res;
+ const char *paramValues[3];
+ int paramLengths[3];
+ int paramFormats[3];
+ uint32_t binaryIntVal;
+
+ /*
+ * If the user supplies a parameter on the command line, use it as the
+ * conninfo string; otherwise default to setting dbname=postgres and using
+ * environment variables or defaults for all other connection parameters.
+ */
+ if (argc > 1)
+ conninfo = argv[1];
+ else
+ conninfo = "dbname = postgres";
+
+ /* Make a connection to the database */
+ conn = PQconnectdb(conninfo);
+
+ /* Check to see that the backend connection was successfully made */
+ if (PQstatus(conn) != CONNECTION_OK)
+ {
+ fprintf(stderr, "Connection to database failed: %s",
+ PQerrorMessage(conn));
+ exit_nicely(conn);
+ }
+
+ /* Set always-secure search path, so malicious users can't take control. */
+ res = PQexec(conn,
+ "SELECT pg_catalog.set_config('search_path', '', false)");
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ {
+ fprintf(stderr, "SET failed: %s", PQerrorMessage(conn));
+ PQclear(res);
+ exit_nicely(conn);
+ }
+ PQclear(res);
+
+ /*
+ * Transmit parameters in different forms and make a statement fail. User
+ * can then verify the server log.
+ */
+ res = PQexec(conn, "SET log_parameters_on_error = on");
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ {
+ fprintf(stderr, "SET failed: %s", PQerrorMessage(conn));
+ PQclear(res);
+ exit_nicely(conn);
+ }
+ PQclear(res);
+
+ /* emit error on parse stage */
+ paramValues[0] = (char *) "non-number -- parse";
+ paramLengths[0] = strlen(paramValues[2]) + 1;
+ paramFormats[0] = 0; /* text */
+ res = PQexecParams(conn,
+ "SELECT $1::int",
+ 1, /* # params */
+ NULL, /* let the backend deduce param type */
+ paramValues,
+ paramLengths,
+ paramFormats,
+ 1); /* ask for binary results */
+
+ if (PQresultStatus(res) != PGRES_FATAL_ERROR)
+ {
+ fprintf(stderr, "SELECT succeeded but was supposed to fail\n");
+ PQclear(res);
+ exit_nicely(conn);
+ }
+ PQclear(res);
+ printf("text->int conversion error expected on parse stage, "
+ "got an error message from server: %s\n",
+ PQerrorMessage(conn));
+
+ /* emit error on bind stage */
+ paramValues[0] = (char *) "{malformed json -- bind :-{";
+ paramLengths[0] = strlen(paramValues[2]) + 1;
+ paramFormats[0] = 0; /* text */
+ res = PQexecParams(conn,
+ "SELECT (' ' || $1::text || ' ')::json",
+ 1, /* # params */
+ NULL, /* let the backend deduce param type */
+ paramValues,
+ paramLengths,
+ paramFormats,
+ 1); /* ask for binary results */
+
+ if (PQresultStatus(res) != PGRES_FATAL_ERROR)
+ {
+ fprintf(stderr, "SELECT succeeded but was supposed to fail\n");
+ PQclear(res);
+ exit_nicely(conn);
+ }
+ PQclear(res);
+ printf("Json parsing error expected on bind stage, "
+ "got an error message from server: %s\n"
+ "Please make sure it has been logged "
+ "with bind parameters in server log\n",
+ PQerrorMessage(conn));
+
+ /* divide by zero -- but server won't realize until execution; 0 params */
+ res = PQexecParams(conn,
+ "SELECT 1 / (random() / 2)::int /*exec*/",
+ 0, /* # params */
+ NULL, /* let the backend deduce param type */
+ NULL,
+ NULL,
+ NULL,
+ 1); /* ask for binary results */
+
+ if (PQresultStatus(res) != PGRES_FATAL_ERROR)
+ {
+ fprintf(stderr, "SELECT succeeded but was supposed to fail\n");
+ PQclear(res);
+ exit_nicely(conn);
+ }
+ PQclear(res);
+ printf("Division by zero on execution expected, "
+ "got an error message from server: %s\n"
+ "Please make sure it has been logged "
+ "with bind parameters in server log\n",
+ PQerrorMessage(conn));
+
+ /* divide by zero -- but server won't realize until execution; 3 params */
+ paramValues[0] = (char *) &binaryIntVal;
+ paramLengths[0] = sizeof(binaryIntVal);
+ paramFormats[0] = 1; /* binary */
+ paramValues[1] = "2";
+ paramLengths[1] = strlen(paramValues[1]) + 1;
+ paramFormats[1] = 0; /* text */
+ paramValues[2] = (char *) "everyone's little $$ in \"dollar\"";
+ paramLengths[2] = strlen(paramValues[2]) + 1;
+ paramFormats[2] = 0; /* text */
+ res = PQexecParams(
+ conn,
+ "SELECT 1 / (random() / 2)::int + $1::int + $2::int, $3::text /*exec*/",
+ 3, /* # params */
+ NULL, /* let the backend deduce param type */
+ paramValues,
+ paramLengths,
+ paramFormats,
+ 1 /* ask for binary results */
+ );
+
+ if (PQresultStatus(res) != PGRES_FATAL_ERROR)
+ {
+ fprintf(stderr, "SELECT succeeded but was supposed to fail\n");
+ PQclear(res);
+ exit_nicely(conn);
+ }
+ PQclear(res);
+ printf("Division by zero on execution expected, "
+ "got an error message from server: %s\n"
+ "Please make sure it has been logged "
+ "with bind parameters in server log\n",
+ PQerrorMessage(conn));
+
+ /* close the connection to the database and cleanup */
+ PQfinish(conn);
+
+ return 0;
+}
