Date: Mon, 4 Jan 2021 11:09:39 -0600
From: Justin Pryzby <pryzby@telsasoft.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Alexey Bashtanov <bashtanov@imap.cc>, Franck Verrot <franck@verrot.fr>,
	pgsql-hackers@postgresql.org
Subject: [PATCH]: Allow errors in parameter values to be reported during the
 BIND phase itself..
Message-ID: <20210104170939.GH9712@telsasoft.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="d01dLTUuW90fS44H"
Content-Disposition: inline
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk


--d01dLTUuW90fS44H
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

For example:

$ python3.5 -c "import pg; db=pg.DB(); q = db.query(\"SET log_parameter_max_length_on_error=-1;\"); db.prepare('p', 'SELECT \$1::smallint'); db.query_prepared('p',66666);"
2021-01-03 02:21:04.547 CST [20157] ERROR:  value "66666" is out of range for type smallint
2021-01-03 02:21:04.547 CST [20157] CONTEXT:  unnamed portal with parameters: $1 = '66666'
2021-01-03 02:21:04.547 CST [20157] STATEMENT:  SELECT $1::smallint

When there are many bind params, this can be useful to determine which is out
of range.  Think 900 int/smallint columns, or less-wide tables being inserted
multiple rows at a time with VALUES(),(),()...

Of course, this isn't as good as showing the column name, so I might pursue
Tom's suggestion for that at some point.

-- 
Justin

--d01dLTUuW90fS44H
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0001-Allow-errors-in-parameter-values-to-be-reported-duri.patch"

From 40dd798d8518b4d113dbf70c37687c87bf734659 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Fri, 1 Jan 2021 19:38:57 -0600
Subject: [PATCH] Allow errors in parameter values to be reported during the
 BIND phase itself..

For example:
$ python3.5 -c "import pg,time; db=pg.DB('dbname=postgres host=/var/run/postgresql port=5432 host=/tmp'); q = db.query(\"SET log_parameter_max_length_on_error=-1;\"); db.prepare('p', 'SELECT \$1::smallint'); db.query_prepared('p',66666);"
2021-01-01 19:40:24.777 CST [5330] ERROR:  value "66666" is out of range for type smallint
2021-01-01 19:40:24.777 CST [5330] CONTEXT:  unnamed portal with parameters: $1 = '66666'

This can be useful to determine which is out of range when there are many bind
params.

See also:
commit ba79cb5dc
commit 0b34e7d30
https://www.postgresql.org/message-id/flat/CANfkH5k-6nNt-4cSv1vPB80nq2BZCzhFVR5O4VznYbsX0wZmow@mail.gmail.com
  Mention column name in error messages
---
 src/backend/tcop/postgres.c | 166 +++++++++++++++++++++++-------------
 1 file changed, 106 insertions(+), 60 deletions(-)

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 317d1aa573..30fb86d97f 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -1765,9 +1765,109 @@ exec_bind_message(StringInfo input_message)
 	if (numParams > 0)
 	{
 		char	  **knownTextValues = NULL; /* allocate on first use */
+		int32		*plengths;
+		char	  **pstrings;
+
+		plengths = palloc0(numParams * sizeof(*plengths));
+
+		/* Indexed by paramno, but not used for nulls */
+		pstrings = palloc0(numParams * sizeof(*pstrings));
 
 		params = makeParamList(numParams);
 
+		/*
+		 * Do two loops to allow error reports during BIND;
+		 * On the first pass, just save the offset and length of each param;
+		 * On the second pass, convert the params to the require type.
+		 * If this fails, the params have already been saved for error callack.
+		 */
+
+		for (int paramno = 0; paramno < numParams; paramno++)
+		{
+			int32		plength;
+			int16		pformat;
+			char	   *pstring;
+			MemoryContext oldcxt;
+
+			plength = plengths[paramno] = pq_getmsgint(input_message, 4);
+
+			/* Minimal amount needed for error callback */
+			params->params[paramno].ptype = psrc->param_types[paramno];;
+			params->params[paramno].isnull = (plength == -1);
+
+			if (params->params[paramno].isnull)
+				continue;
+
+			pstrings[paramno] = unconstify(char *, pq_getmsgbytes(input_message, plength));
+
+			if (log_parameter_max_length_on_error == 0)
+				continue;
+
+			if (numPFormats > 1)
+				pformat = pformats[paramno];
+			else if (numPFormats > 0)
+				pformat = pformats[0];
+			else
+				pformat = 0;	/* default = text */
+
+			if (pformat != 0)	/* text mode */
+				continue;
+
+			/*
+			 * If we might need to log parameters later, save a copy of
+			 * the converted string in MessageContext;
+			 * XXX: only supported in text mode ??
+			 */
+			pstring = pg_client_to_server(pstrings[paramno], plength);
+			oldcxt = MemoryContextSwitchTo(MessageContext);
+
+			if (knownTextValues == NULL)
+				knownTextValues =
+					palloc0(numParams * sizeof(char *));
+
+			if (log_parameter_max_length_on_error < 0)
+				knownTextValues[paramno] = pstrdup(pstring);
+			else
+			{
+				/*
+				 * We can trim the saved string, knowing that we
+				 * won't print all of it.  But we must copy at
+				 * least two more full characters than
+				 * BuildParamLogString wants to use; otherwise it
+				 * might fail to include the trailing ellipsis.
+				 */
+				knownTextValues[paramno] =
+					pnstrdup(pstring,
+							 log_parameter_max_length_on_error
+							 + 2 * MAX_MULTIBYTE_CHAR_LEN);
+			}
+
+			MemoryContextSwitchTo(oldcxt);
+			if (pstring != pstrings[paramno])
+				pfree(pstring);
+		}
+
+		/*
+		 * Once all parameters have been received, prepare for printing them
+		 * in errors, if configured to do so.  (This is saved in the portal,
+		 * so that they'll appear when the query is executed later.)
+		 */
+		if (log_parameter_max_length_on_error != 0)
+		{
+			params->paramValuesStr =
+				BuildParamLogString(params,
+									knownTextValues,
+									log_parameter_max_length_on_error);
+		}
+
+		/* Set the error callback so that parameters are logged, as needed  */
+		params_data.portalName = portal->name;
+		params_data.params = params;
+		params_errcxt.previous = error_context_stack;
+		params_errcxt.callback = ParamsErrorCallback;
+		params_errcxt.arg = (void *) &params_data;
+		error_context_stack = &params_errcxt;
+
 		for (int paramno = 0; paramno < numParams; paramno++)
 		{
 			Oid			ptype = psrc->param_types[paramno];
@@ -1778,13 +1878,11 @@ exec_bind_message(StringInfo input_message)
 			char		csave;
 			int16		pformat;
 
-			plength = pq_getmsgint(input_message, 4);
+			plength = plengths[paramno];
 			isNull = (plength == -1);
 
 			if (!isNull)
 			{
-				const char *pvalue = pq_getmsgbytes(input_message, plength);
-
 				/*
 				 * Rather than copying data around, we just set up a phony
 				 * StringInfo pointing to the correct portion of the message
@@ -1793,7 +1891,7 @@ exec_bind_message(StringInfo input_message)
 				 * trailing null.  This is grotty but is a big win when
 				 * dealing with very large parameter strings.
 				 */
-				pbuf.data = unconstify(char *, pvalue);
+				pbuf.data = pstrings[paramno];
 				pbuf.maxlen = plength + 1;
 				pbuf.len = plength;
 				pbuf.cursor = 0;
@@ -1833,45 +1931,9 @@ exec_bind_message(StringInfo input_message)
 
 				pval = OidInputFunctionCall(typinput, pstring, typioparam, -1);
 
-				/*
-				 * If we might need to log parameters later, save a copy of
-				 * the converted string in MessageContext; then free the
-				 * result of encoding conversion, if any was done.
-				 */
-				if (pstring)
-				{
-					if (log_parameter_max_length_on_error != 0)
-					{
-						MemoryContext oldcxt;
-
-						oldcxt = MemoryContextSwitchTo(MessageContext);
-
-						if (knownTextValues == NULL)
-							knownTextValues =
-								palloc0(numParams * sizeof(char *));
-
-						if (log_parameter_max_length_on_error < 0)
-							knownTextValues[paramno] = pstrdup(pstring);
-						else
-						{
-							/*
-							 * We can trim the saved string, knowing that we
-							 * won't print all of it.  But we must copy at
-							 * least two more full characters than
-							 * BuildParamLogString wants to use; otherwise it
-							 * might fail to include the trailing ellipsis.
-							 */
-							knownTextValues[paramno] =
-								pnstrdup(pstring,
-										 log_parameter_max_length_on_error
-										 + 2 * MAX_MULTIBYTE_CHAR_LEN);
-						}
-
-						MemoryContextSwitchTo(oldcxt);
-					}
-					if (pstring != pbuf.data)
+				/* free the result of encoding conversion, if any was done. */
+				if (pstring && pstring != pbuf.data)
 						pfree(pstring);
-				}
 			}
 			else if (pformat == 1)	/* binary mode */
 			{
@@ -1922,16 +1984,8 @@ exec_bind_message(StringInfo input_message)
 			params->params[paramno].ptype = ptype;
 		}
 
-		/*
-		 * Once all parameters have been received, prepare for printing them
-		 * in errors, if configured to do so.  (This is saved in the portal,
-		 * so that they'll appear when the query is executed later.)
-		 */
-		if (log_parameter_max_length_on_error != 0)
-			params->paramValuesStr =
-				BuildParamLogString(params,
-									knownTextValues,
-									log_parameter_max_length_on_error);
+		pfree(plengths);
+		pfree(pstrings);
 	}
 	else
 		params = NULL;
@@ -1939,14 +1993,6 @@ exec_bind_message(StringInfo input_message)
 	/* Done storing stuff in portal's context */
 	MemoryContextSwitchTo(oldContext);
 
-	/* Set the error callback so that parameters are logged, as needed  */
-	params_data.portalName = portal->name;
-	params_data.params = params;
-	params_errcxt.previous = error_context_stack;
-	params_errcxt.callback = ParamsErrorCallback;
-	params_errcxt.arg = (void *) &params_data;
-	error_context_stack = &params_errcxt;
-
 	/* Get the result format codes */
 	numRFormats = pq_getmsgint(input_message, 2);
 	if (numRFormats > 0)
-- 
2.17.0


--d01dLTUuW90fS44H--