From ba79cb5dc841104cf4810b5c23af4f881079dbb5 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Wed, 11 Dec 2019 18:03:35 -0300 Subject: Emit parameter values during query bind/execute errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes such log entries more useful, since the cause of the error can be dependent on the parameter values. Author: Alexey Bashtanov, Álvaro Herrera Discussion: https://postgr.es/m/0146a67b-a22a-0519-9082-bc29756b93a2@imap.cc Reviewed-by: Peter Eisentraut, Andres Freund, Tom Lane --- src/backend/tcop/postgres.c | 119 +++++++++++++++++++++++++++----------------- 1 file changed, 72 insertions(+), 47 deletions(-) (limited to 'src/backend/tcop/postgres.c') diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 512209a38c4..0b7bc1fd030 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -1614,6 +1614,8 @@ exec_bind_message(StringInfo input_message) bool save_log_statement_stats = log_statement_stats; bool snapshot_set = false; char msec_str[32]; + ParamsErrorCbData params_data; + ErrorContextCallback params_errcxt; /* Get the fixed part of the message */ portal_name = pq_getmsgstring(input_message); @@ -1753,6 +1755,8 @@ exec_bind_message(StringInfo input_message) */ if (numParams > 0) { + char **knownTextValues = NULL; /* allocate on first use */ + params = makeParamList(numParams); for (int paramno = 0; paramno < numParams; paramno++) @@ -1820,9 +1824,32 @@ 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); + /* + * Free result of encoding conversion, if any, and save a copy + * for later when logging parameters. + */ + if (pstring) + { + if (log_parameters_on_error) + { + MemoryContext oldcxt; + + oldcxt = MemoryContextSwitchTo(MessageContext); + if (knownTextValues == NULL) + knownTextValues = + palloc0(numParams * sizeof(char *)); + /* + * Note: must copy at least two more full characters + * than BuildParamLogString wants to see; otherwise it + * might fail to include the ellipsis. + */ + knownTextValues[paramno] = + pnstrdup(pstring, 64 + 2 * MAX_MULTIBYTE_CHAR_LEN); + MemoryContextSwitchTo(oldcxt); + } + if (pstring != pbuf.data) + pfree(pstring); + } } else if (pformat == 1) /* binary mode */ { @@ -1872,6 +1899,15 @@ exec_bind_message(StringInfo input_message) params->params[paramno].pflags = PARAM_FLAG_CONST; 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_parameters_on_error) + params->paramValuesStr = + BuildParamLogString(params, knownTextValues, 64); } else params = NULL; @@ -1879,6 +1915,14 @@ 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) @@ -1924,6 +1968,12 @@ exec_bind_message(StringInfo input_message) */ PortalSetResultFormat(portal, numRFormats, rformats); + /* + * Done binding; remove the parameters error callback. Entries emitted + * later determine independently whether to log the parameters or not. + */ + error_context_stack = error_context_stack->previous; + /* * Send BindComplete. */ @@ -1980,6 +2030,8 @@ exec_execute_message(const char *portal_name, long max_rows) bool execute_is_fetch; bool was_logged = false; char msec_str[32]; + ParamsErrorCbData params_data; + ErrorContextCallback params_errcxt; /* Adjust destination to tell printtup.c what to do */ dest = whereToSendOutput; @@ -2104,8 +2156,16 @@ exec_execute_message(const char *portal_name, long max_rows) CHECK_FOR_INTERRUPTS(); /* - * Okay to run the portal. + * Okay to run the portal. Set the error callback so that parameters are + * logged. The parameters must have been saved during the bind phase. */ + params_data.portalName = portal->name; + params_data.params = portalParams; + params_errcxt.previous = error_context_stack; + params_errcxt.callback = ParamsErrorCallback; + params_errcxt.arg = (void *) ¶ms_data; + error_context_stack = ¶ms_errcxt; + if (max_rows <= 0) max_rows = FETCH_ALL; @@ -2119,6 +2179,9 @@ exec_execute_message(const char *portal_name, long max_rows) receiver->rDestroy(receiver); + /* Done executing; remove the params error callback */ + error_context_stack = error_context_stack->previous; + if (completed) { if (is_xact_command) @@ -2329,51 +2392,13 @@ 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()) + if (params && params->numParams > 0) { - StringInfoData param_str; - MemoryContext oldcontext; + char *str; - /* This code doesn't support dynamic param lists */ - Assert(params->paramFetch == NULL); - - /* Make sure any trash is generated in MessageContext */ - oldcontext = MemoryContextSwitchTo(MessageContext); - - initStringInfo(¶m_str); - - for (int paramno = 0; paramno < params->numParams; paramno++) - { - ParamExternData *prm = ¶ms->params[paramno]; - Oid typoutput; - bool typisvarlena; - char *pstring; - - appendStringInfo(¶m_str, "%s$%d = ", - paramno > 0 ? ", " : "", - paramno + 1); - - if (prm->isnull || !OidIsValid(prm->ptype)) - { - appendStringInfoString(¶m_str, "NULL"); - continue; - } - - getTypeOutputInfo(prm->ptype, &typoutput, &typisvarlena); - - pstring = OidOutputFunctionCall(typoutput, prm->value); - - appendStringInfoStringQuoted(¶m_str, pstring, 0); - - pfree(pstring); - } - - errdetail("parameters: %s", param_str.data); - - pfree(param_str.data); - - MemoryContextSwitchTo(oldcontext); + str = BuildParamLogString(params, NULL, 0); + if (str && str[0] != '\0') + errdetail("parameters: %s", str); } return 0; -- cgit v1.2.3