Subject: Re: log bind parameter values on error
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
 Andres Freund <andres@anarazel.de>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>
References: <20190918205853.GA21408@alvherre.pgsql>
From: Alexey Bashtanov <bashtanov@imap.cc>
Message-ID: <e5196395-cf41-587c-331f-95bb38e3a11e@imap.cc>
Date: Fri, 27 Sep 2019 17:45:23 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <20190918205853.GA21408@alvherre.pgsql>
Content-Type: multipart/mixed;
 boundary="------------989DDBBB8D2EF5B28F98A855"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------989DDBBB8D2EF5B28F98A855
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit

Hello Alvaro,

> I didn't like abusing testlibpq3.c for your new stuff, so I moved it off
> to a new file testlibpq5.c.  I cleaned up a few other cosmetics things
> about this -- v10 attached.
Thanks for doing this.

> I eventually noticed that this patch fails
> to initialize each param's textValue to NULL,
I've added the changes to set textValue to NULL for each parameter after 
calling makeParamList.
I think it's the best option, as it's natural to populate text 
representations (albeit with NULLs)
in the same loop as we write the other parameters attributes.

It still requires us to set hasTextValues afterwards, as it doesn't seem 
practical to me to make
two loops, first null them all and next populate them: if there's a 
parsing or converting problem
midway it's out of the feature scope and already being logged elsewhere.

Attaching v11, marking as waiting for review.

Best, Alex

--------------989DDBBB8D2EF5B28F98A855
Content-Type: text/x-patch;
 name="log_parameters_v011.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="log_parameters_v011.patch"

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 6612f95f9f..81dfee5fe7 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6414,6 +6414,23 @@ 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 the statement is logged with bind parameter values. 
+        It adds some overhead, as postgres will cache textual
+        representations of parameter values in memory for all statements,
+        even if they eventually do not get logged. 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/commands/prepare.c b/src/backend/commands/prepare.c
index 7e0a041fab..7692ea551b 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -406,7 +406,7 @@ EvaluateParams(PreparedStatement *pstmt, List *params,
 		prm->value = ExecEvalExprSwitchContext(n,
 											   GetPerTupleExprContext(estate),
 											   &prm->isnull);
-
+		prm->textValue = NULL;
 		i++;
 	}
 
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 83337c2eed..92fcee8b19 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -925,6 +925,7 @@ postquel_sub_params(SQLFunctionCachePtr fcache,
 			prm->isnull = fcinfo->args[i].isnull;
 			prm->pflags = 0;
 			prm->ptype = fcache->pinfo->argtypes[i];
+			prm->textValue = NULL;
 		}
 	}
 	else
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 2c0ae395ba..f6b4246b47 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -2458,6 +2458,7 @@ _SPI_convert_params(int nargs, Oid *argtypes,
 			prm->isnull = (Nulls && Nulls[i] == 'n');
 			prm->pflags = PARAM_FLAG_CONST;
 			prm->ptype = argtypes[i];
+			prm->textValue = NULL;
 		}
 	}
 	else
diff --git a/src/backend/nodes/params.c b/src/backend/nodes/params.c
index cf4387e40f..bf8e3705af 100644
--- a/src/backend/nodes/params.c
+++ b/src/backend/nodes/params.c
@@ -45,6 +45,7 @@ makeParamList(int numParams)
 	retval->parserSetup = NULL;
 	retval->parserSetupArg = NULL;
 	retval->numParams = numParams;
+	retval->hasTextValues = false;
 
 	return retval;
 }
@@ -91,6 +92,9 @@ copyParamList(ParamListInfo from)
 			continue;
 		get_typlenbyval(nprm->ptype, &typLen, &typByVal);
 		nprm->value = datumCopy(nprm->value, typByVal, typLen);
+
+		/* We don't copy text values, as no caller needs that at present. */
+		nprm->textValue = NULL;
 	}
 
 	return retval;
@@ -247,6 +251,9 @@ RestoreParamList(char **start_address)
 
 		/* Read datum/isnull. */
 		prm->value = datumRestore(start_address, &prm->isnull);
+
+		/* We don't copy text values, as no caller needs that at present. */
+		prm->textValue = NULL;
 	}
 
 	return paramLI;
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index e8d8e6f828..ec1e4092f5 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -86,6 +86,12 @@
  */
 const char *debug_query_string; /* client-supplied query string */
 
+/*
+ * The top-level portal that the client is immediately working with:
+ * creating, binding, executing, or all at one using simple protocol
+ */
+Portal current_top_portal = NULL;
+
 /* Note: whereToSendOutput is initialized for the bootstrap/standalone case */
 CommandDest whereToSendOutput = DestDebug;
 
@@ -1714,6 +1720,9 @@ exec_bind_message(StringInfo input_message)
 	else
 		portal = CreatePortal(portal_name, false, false);
 
+	Assert(current_top_portal == NULL);
+	current_top_portal = portal;
+
 	/*
 	 * Prepare to copy stuff into the portal's memory context.  We do all this
 	 * copying first, because it could possibly fail (out-of-memory) and we
@@ -1751,6 +1760,9 @@ exec_bind_message(StringInfo input_message)
 	 */
 	if (numParams > 0)
 	{
+		/* GUC value can change, so we remember its state to be consistent */
+		bool need_text_values = log_parameters_on_error;
+
 		params = makeParamList(numParams);
 
 		for (int paramno = 0; paramno < numParams; paramno++)
@@ -1818,9 +1830,18 @@ 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, keep it -- possibly making
+				 * a copy first, if necessary.  Otherwise get rid of it.
+				 */
+				if (pstring)
+				{
+					if (need_text_values)
+						params->params[paramno].textValue =
+							pstring == pbuf.data ? pstrdup(pstring) : pstring;
+					else if (pstring != pbuf.data)
+						pfree(pstring);
+				}
 			}
 			else if (pformat == 1)	/* binary mode */
 			{
@@ -1846,6 +1867,23 @@ exec_bind_message(StringInfo input_message)
 							(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
 							 errmsg("incorrect binary data format in bind parameter %d",
 									paramno + 1)));
+
+				/*
+				 * Compute textual representation for further logging.
+				 *
+				 * This could be optimized for certain built-in types, for
+				 * which the output function is safe to call even in an
+				 * aborted transaction.
+				 */
+				if (!isNull && need_text_values)
+				{
+					Oid			typoutput;
+					bool		typisvarlena;
+
+					getTypeOutputInfo(ptype, &typoutput, &typisvarlena);
+					params->params[paramno].textValue =
+						OidOutputFunctionCall(typoutput, pval);
+				}
 			}
 			else
 			{
@@ -1870,10 +1908,19 @@ exec_bind_message(StringInfo input_message)
 			params->params[paramno].pflags = PARAM_FLAG_CONST;
 			params->params[paramno].ptype = ptype;
 		}
+
+		/* Only now can we safely set this. */
+		params->hasTextValues = need_text_values;
 	}
 	else
 		params = NULL;
 
+	/*
+	 * Set portal parameters early for them to get logged if an error happens
+	 * on planning stage
+	 */
+	portal->portalParams = params;
+
 	/* Done storing stuff in portal's context */
 	MemoryContextSwitchTo(oldContext);
 
@@ -1954,6 +2001,7 @@ exec_bind_message(StringInfo input_message)
 	if (save_log_statement_stats)
 		ShowUsage("BIND MESSAGE STATISTICS");
 
+	current_top_portal = NULL;
 	debug_query_string = NULL;
 }
 
@@ -2004,32 +2052,49 @@ exec_execute_message(const char *portal_name, long max_rows)
 	/* Does the portal contain a transaction command? */
 	is_xact_command = IsTransactionStmtList(portal->stmts);
 
-	/*
-	 * We must copy the sourceText and prepStmtName into MessageContext in
-	 * case the portal is destroyed during finish_xact_command. Can avoid the
-	 * copy if it's not an xact command, though.
-	 */
 	if (is_xact_command)
 	{
+		/*
+		 * We must copy the sourceText and prepStmtName into MessageContext in
+		 * case the portal is destroyed during finish_xact_command.
+		 */
 		sourceText = pstrdup(portal->sourceText);
 		if (portal->prepStmtName)
 			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;
+
+		/*
+		 * We don't set current top portal for xact cmds, as
+		 * 1) the portal may be destroyed earlier than the error happens
+		 * 2) it anyway has no parameters in it to log
+		 */
+		Assert(current_top_portal == NULL);
 	}
 	else
 	{
+		/*
+		 * The portal will remain there until we leave the function, so it's
+		 * safe avoid copying and just to refer to the data in its memory
+		 * context instead
+		 */
 		sourceText = portal->sourceText;
 		if (portal->prepStmtName)
 			prepStmtName = portal->prepStmtName;
 		else
 			prepStmtName = "<unnamed>";
+
+		/*
+		 * For non-xact commands, we remember the current portal and its bind
+		 * parameters for further use in logging
+		 */
+		Assert(current_top_portal == NULL);
+		current_top_portal = portal;
 		portalParams = portal->portalParams;
 	}
 
@@ -2178,6 +2243,7 @@ exec_execute_message(const char *portal_name, long max_rows)
 	if (save_log_statement_stats)
 		ShowUsage("EXECUTE MESSAGE STATISTICS");
 
+	current_top_portal = NULL;
 	debug_query_string = NULL;
 }
 
@@ -2306,61 +2372,21 @@ errdetail_execute(List *raw_parsetree_list)
 static int
 errdetail_params(ParamListInfo params)
 {
-	/* We mustn't call user-defined I/O functions when in an aborted xact */
-	if (params && params->numParams > 0 && !IsAbortedTransactionBlockState())
-	{
-		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(&param_str);
-
-		for (int paramno = 0; paramno < params->numParams; paramno++)
-		{
-			ParamExternData *prm = &params->params[paramno];
-			Oid			typoutput;
-			bool		typisvarlena;
-			char	   *pstring;
-			char	   *p;
-
-			appendStringInfo(&param_str, "%s$%d = ",
-							 paramno > 0 ? ", " : "",
-							 paramno + 1);
-
-			if (prm->isnull || !OidIsValid(prm->ptype))
-			{
-				appendStringInfoString(&param_str, "NULL");
-				continue;
-			}
-
-			getTypeOutputInfo(prm->ptype, &typoutput, &typisvarlena);
-
-			pstring = OidOutputFunctionCall(typoutput, prm->value);
-
-			appendStringInfoCharMacro(&param_str, '\'');
-			for (p = pstring; *p; p++)
-			{
-				if (*p == '\'') /* double single quotes */
-					appendStringInfoCharMacro(&param_str, *p);
-				appendStringInfoCharMacro(&param_str, *p);
-			}
-			appendStringInfoCharMacro(&param_str, '\'');
-
-			pfree(pstring);
-		}
-
-		errdetail("parameters: %s", param_str.data);
+	MemoryContext oldcontext;
+	char	   *params_message;
 
-		pfree(param_str.data);
+	/* Make sure any trash is generated in MessageContext */
+	oldcontext = MemoryContextSwitchTo(MessageContext);
+	params_message = get_portal_bind_params(params);
 
-		MemoryContextSwitchTo(oldcontext);
+	if (params_message)
+	{
+		errdetail("parameters: %s", params_message);
+		pfree(params_message);
 	}
 
+	MemoryContextSwitchTo(oldcontext);
+
 	return 0;
 }
 
@@ -2678,6 +2704,86 @@ drop_unnamed_stmt(void)
 	}
 }
 
+/*
+ * get_portal_bind_params
+ * 		Get a string containing parameters data -- used for logging.
+ *
+ * Can return NULL if there are no parameters in the portal or the portal is
+ * not valid, or the text representations of the parameters are not available.
+ * If returning a non-NULL value, it allocates memory for the returned string
+ * in the current context, and it's the caller's responsibility to pfree() it.
+ */
+char *
+get_portal_bind_params(ParamListInfo params)
+{
+	StringInfoData param_str;
+
+	/* No parameters to format */
+	if (!params || params->numParams == 0)
+		return NULL;
+
+	/*
+	 * We either need textual representation of parameters pre-calculated, or
+	 * call potentially user-defined I/O functions to convert the internal
+	 * representation into text, which cannot be done in an aborted xact
+	 */
+	if (!params->hasTextValues && IsAbortedTransactionBlockState())
+		return NULL;
+
+	initStringInfo(&param_str);
+
+	/* This code doesn't support dynamic param lists */
+	Assert(params->paramFetch == NULL);
+
+	for (int paramno = 0; paramno < params->numParams; paramno++)
+	{
+		ParamExternData *prm = &params->params[paramno];
+		char	   *pstring;
+		char	   *p;
+
+		appendStringInfo(&param_str, "%s$%d = ",
+						 paramno > 0 ? ", " : "",
+						 paramno + 1);
+
+		if (prm->isnull)
+		{
+			appendStringInfoString(&param_str, "NULL");
+			continue;
+		}
+
+		if (params->hasTextValues)
+			pstring = prm->textValue;
+		else
+		{
+			Oid			typoutput;
+			bool		typisvarlena;
+
+			if (!OidIsValid(prm->ptype))
+			{
+				appendStringInfoString(&param_str, "UNKNOWN TYPE");
+				continue;
+			}
+
+			getTypeOutputInfo(prm->ptype, &typoutput, &typisvarlena);
+			pstring = OidOutputFunctionCall(typoutput, prm->value);
+		}
+
+		appendStringInfoCharMacro(&param_str, '\'');
+		for (p = pstring; *p; p++)
+		{
+			if (*p == '\'') /* double single quotes */
+				appendStringInfoCharMacro(&param_str, *p);
+			appendStringInfoCharMacro(&param_str, *p);
+		}
+		appendStringInfoCharMacro(&param_str, '\'');
+
+		if (!params->hasTextValues)
+			pfree(pstring);
+	}
+
+	return param_str.data;
+}
+
 
 /* --------------------------------
  *		signal handler routines used in PostgresMain()
@@ -4031,10 +4137,11 @@ PostgresMain(int argc, char *argv[],
 		EmitErrorReport();
 
 		/*
-		 * Make sure debug_query_string gets reset before we possibly clobber
-		 * the storage it points at.
+		 * Make sure these get reset before we possibly clobber
+		 * the storages they point at.
 		 */
 		debug_query_string = NULL;
+		current_top_portal = NULL;
 
 		/*
 		 * Abort the current transaction in order to recover.
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 8b4720ef3a..40896a2d09 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -76,6 +76,7 @@
 #include "tcop/tcopprot.h"
 #include "utils/guc.h"
 #include "utils/memutils.h"
+#include "utils/portal.h"
 #include "utils/ps_status.h"
 
 
@@ -344,6 +345,7 @@ errstart(int elevel, const char *filename, int lineno,
 		{
 			error_context_stack = NULL;
 			debug_query_string = NULL;
+			current_top_portal = NULL;
 		}
 	}
 	if (++errordata_stack_depth >= ERRORDATA_STACK_SIZE)
@@ -2788,6 +2790,19 @@ write_csvlog(ErrorData *edata)
 	if (print_stmt && edata->cursorpos > 0)
 		appendStringInfo(&buf, "%d", edata->cursorpos);
 	appendStringInfoChar(&buf, ',');
+	if (print_stmt && log_parameters_on_error &&
+		PortalIsValid(current_top_portal))
+	{
+		char *params;
+
+		params = get_portal_bind_params(current_top_portal->portalParams);
+		if (params != NULL)
+		{
+			appendCSVLiteral(&buf, params);
+			pfree(params);
+		}
+	}
+	appendStringInfoChar(&buf, ',');
 
 	/* file error location */
 	if (Log_error_verbosity >= PGERROR_VERBOSE)
@@ -2944,6 +2959,19 @@ send_message_to_server_log(ErrorData *edata)
 		appendStringInfoString(&buf, _("STATEMENT:  "));
 		append_with_tabs(&buf, debug_query_string);
 		appendStringInfoChar(&buf, '\n');
+
+		if (log_parameters_on_error && PortalIsValid(current_top_portal))
+		{
+			char *params;
+
+			params = get_portal_bind_params(current_top_portal->portalParams);
+			if (params != NULL)
+			{
+				log_line_prefix(&buf, edata);
+				appendStringInfo(&buf, _("PARAMETERS:  %s\n"), params);
+				pfree(params);
+			}
+		}
 	}
 
 #ifdef HAVE_SYSLOG
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 2178e1cf5e..c034f88bfa 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -483,6 +483,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;
@@ -1293,6 +1294,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 0fc23e3a61..fccfff3a03 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -532,6 +532,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/include/nodes/params.h b/src/include/nodes/params.h
index fd9046619c..316d3a2888 100644
--- a/src/include/nodes/params.h
+++ b/src/include/nodes/params.h
@@ -93,6 +93,7 @@ typedef struct ParamExternData
 	bool		isnull;			/* is it NULL? */
 	uint16		pflags;			/* flag bits, see above */
 	Oid			ptype;			/* parameter's datatype, or 0 */
+	char	   *textValue;		/* textual representation for debug purposes */
 } ParamExternData;
 
 typedef struct ParamListInfoData *ParamListInfo;
@@ -116,6 +117,8 @@ typedef struct ParamListInfoData
 	ParserSetupHook parserSetup;	/* parser setup hook */
 	void	   *parserSetupArg;
 	int			numParams;		/* nominal/maximum # of Params represented */
+	bool		hasTextValues;	/* whether textValue for all non-null
+														params is populated */
 
 	/*
 	 * params[] may be of length zero if paramFetch is supplied; otherwise it
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h
index ec21f7e45c..ad6517414b 100644
--- a/src/include/tcop/tcopprot.h
+++ b/src/include/tcop/tcopprot.h
@@ -19,6 +19,7 @@
 #include "nodes/plannodes.h"
 #include "storage/procsignal.h"
 #include "utils/guc.h"
+#include "utils/portal.h"
 #include "utils/queryenvironment.h"
 
 
@@ -27,6 +28,8 @@
 
 extern CommandDest whereToSendOutput;
 extern PGDLLIMPORT const char *debug_query_string;
+extern PGDLLIMPORT Portal current_top_portal;
+
 extern int	max_stack_depth;
 extern int	PostAuthDelay;
 
@@ -78,6 +81,7 @@ extern long get_stack_depth_rlimit(void);
 extern void ResetUsage(void);
 extern void ShowUsage(const char *title);
 extern int	check_log_duration(char *msec_str, bool was_logged);
+extern char *get_portal_bind_params(ParamListInfo params);
 extern void set_debug_options(int debug_flag,
 							  GucContext context, GucSource source);
 extern bool set_plan_disabling_options(const char *arg,
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 6791e0cbc2..57acc28ef9 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
index e69de29bb2..69b65c512b 100644
--- a/src/test/examples/testlibpq5.c
+++ b/src/test/examples/testlibpq5.c
@@ -0,0 +1,116 @@
+/*
+ * 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);
+
+	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 */
+	/* divide by zero -- but server won't realize until execution */
+	res = PQexecParams(conn,
+					   "SELECT 1 / (random() / 2)::int + $1::int + $2::int, $3::text",
+					   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");
+		PQclear(res);
+		exit_nicely(conn);
+	}
+	PQclear(res);
+	printf("Division by zero 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;
+}

--------------989DDBBB8D2EF5B28F98A855--