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 <pryz...@telsasoft.com> 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