From: Alexey Bashtanov <bashtanov@imap.cc>
Subject: Re: log bind parameter values on error
To: Andres Freund <andres@anarazel.de>,
 Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>
References: <20190920174627.qmaditduxlrrzb5l@alap3.anarazel.de>
 <20190920195657.GA20546@alvherre.pgsql>
 <20190920203905.xkv5udsd5dxfs6tr@alap3.anarazel.de>
Message-ID: <4e374af0-5a2b-73fa-3dfe-d180fcd2ee3e@imap.cc>
Date: Tue, 5 Nov 2019 12:07:50 +0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.9.0
MIME-Version: 1.0
In-Reply-To: <20190920203905.xkv5udsd5dxfs6tr@alap3.anarazel.de>
Content-Type: multipart/mixed;
 boundary="------------1EFB7554B1CC0E3212742F61"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------1EFB7554B1CC0E3212742F61
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit

Hi Anders and Alvaro,

I must have missed this conversation branch when sending in v011.
Sorry about that.

 > I think it might be worthwhile to cross-reference
 > log_min_error_statement, as log_parameters_on_error will only take 
effect when the
 > statement is logged due to log_min_error_statement.

Agreed, added some clarification.

 > I don't think "cache" is the right descriptor. Usually caching implies
 > trying to make repeated tasks faster, which isn't the case here.

Well, potentially it can, if we set log_min_error_statement to something
lower than error and emit tons of warnings. But it's not the primary use
case, so I just replaced it by the word "save".


> What I'm suggesting is that PortalStart() would build a string
> representation out of the parameter list (using
> ParamExternData.textValue if set, calling the output function
> otherwise), and stash that in the portal.
>
> At the start of all the already existing PG_TRY blocks in pquery.c we
> push an element onto the error_context_stack that adds the errdetail
> with the parameters to the error.  Alternatively we could also add them
> in in the PG_CATCH blocks, but that'd only work for elevel == ERROR
> (i.e. neither FATAL nor non throwing log levels would be able to enrich
> the error).

I'm a bit worried about going this way, as it makes us log
the query and its parameters too far apart in the code,
and it's trickier to make sure we never log parameters without the query.

I think logging the parameters should not be part of error_context_stack,
but rather a primary part of logging facility itself, like statement.
That's because whether we want to log parameters depends
on print_stmt in elog.c. With print_stmt being computed upon edata,
I'm not sure how to work it out nicely.

> Thinking about this: I think the current approach doesn't actually
> handle recursive errors correctly. Even if we fail to emit the error
> message due to the parameter details requiring a lot of memory, we'd
> again do so (and again fail) when handling that OOM error, because
> current_top_portal afaict would still be set.  At the very least this'd
> need to integrate with the recursion_depth logic in elog.c.
We handle it the same way as we do it for debug_query_string itself:

         if (in_error_recursion_trouble())
         {
             error_context_stack = NULL;
             debug_query_string = NULL;
             current_top_portal = NULL;
         }


I'm attaching the patch with docs fixes.

Best regards,
   Alexey


--------------1EFB7554B1CC0E3212742F61
Content-Type: text/x-patch;
 name="log_parameters_v013.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="log_parameters_v013.patch"

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 0191ec84b1..832ebe57a3 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6492,6 +6492,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/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 4bec40aa28..4b648a9f0a 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;
 
@@ -1716,6 +1722,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
@@ -1753,6 +1762,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++)
@@ -1820,9 +1832,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 */
 			{
@@ -1848,6 +1869,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
 			{
@@ -1872,10 +1910,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);
 
@@ -1956,6 +2003,7 @@ exec_bind_message(StringInfo input_message)
 	if (save_log_statement_stats)
 		ShowUsage("BIND MESSAGE STATISTICS");
 
+	current_top_portal = NULL;
 	debug_query_string = NULL;
 }
 
@@ -2006,32 +2054,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;
 	}
 
@@ -2183,6 +2248,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;
 }
 
@@ -2311,61 +2377,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;
 }
 
@@ -2683,6 +2709,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()
@@ -4035,10 +4141,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 31a5ef0474..9ccd2c2bf0 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
new file mode 100644
index 0000000000..69b65c512b
--- /dev/null
+++ 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;
+}

--------------1EFB7554B1CC0E3212742F61--