On Mon, Jan 04, 2021 at 11:09:39AM -0600, Justin Pryzby wrote:
> 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(),(),()...
This fixes a crash when there are zero bind params, and the error context was
popped after not being pushed.
--
Justin
>From 1796775733edec40ad5515270e3b002c202ca23f Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email protected]>
Date: Fri, 1 Jan 2021 19:38:57 -0600
Subject: [PATCH] report errors in parameter values during BIND
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-4csv1vpb80nq2bzczhfvr5o4vznybsx0wz...@mail.gmail.com
Mention column name in error messages
---
src/backend/tcop/postgres.c | 170 +++++++++++++++++++++++-------------
1 file changed, 108 insertions(+), 62 deletions(-)
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index bb5ccb4578..3965f37e85 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -1618,7 +1618,7 @@ exec_bind_message(StringInfo input_message)
Portal portal;
char *query_string;
char *saved_stmt_name;
- ParamListInfo params;
+ ParamListInfo params = NULL;
MemoryContext oldContext;
bool save_log_statement_stats = log_statement_stats;
bool snapshot_set = false;
@@ -1759,15 +1759,115 @@ exec_bind_message(StringInfo input_message)
snapshot_set = true;
}
+ /*
+ * Set the error callback so that parameters are logged, as needed.
+ * This has no effect until params->paramValuesStr is set
+ */
+ params_data.portalName = portal->name;
+ params_data.params = params;
+ params_errcxt.previous = error_context_stack;
+ params_errcxt.callback = ParamsErrorCallback;
+ params_errcxt.arg = (void *) ¶ms_data;
+ error_context_stack = ¶ms_errcxt;
+
/*
* Fetch parameters, if any, and store in the portal's memory context.
*/
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 required 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);
+
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)
- pfree(pstring);
- }
+ /* 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 *) ¶ms_data;
- error_context_stack = ¶ms_errcxt;
-
/* Get the result format codes */
numRFormats = pq_getmsgint(input_message, 2);
if (numRFormats > 0)
--
2.17.0
