Re: log bind parameter values on error

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Alexey Bashtanov <bashtanov(at)imap(dot)cc>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: log bind parameter values on error
Date: 2019-12-05 23:15:50
Message-ID: 20191205231550.GA28677@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2019-Dec-05, Alvaro Herrera wrote:

> > Makes me wonder though, if we ought to invent something similar to the
> > errdata fields we have for schema, table, etc...
>
> Hmm, perhaps we should do that. It avoids the problem altogether.

... then again, I'm not up for writing everything that would be required
to do that. If somebody wants to propose that separately, I volunteer
to review it. But let's not have that block this patch, since this is
already a clear improvement.

So here's v16. I got rid of all that strlen() business of the output
values; instead, preallocate a reasonable guess at the final length (64
bytes for each of the first 128 parameters, plus 16 bytes for for
parameter from 129 to 1024). This boils down to exactly the initial
size of a stringinfo (1024 bytes) when there are 8 parameters or less.
64 bytes avg of guessed length should be plenty (and it will be even
more so when I add the patch to limit the output length to 64+epsilon
bytes per param) I admit there are perhaps too many magical numbers in
those three lines, though.

I don't understand how we ended up with the params put in
errdetail_log() -- seems to have been introduced silently between v13
and v14. What's the reason for that? I put it back as errdetail().
Alexey added some discussion about using context/detail when he sent
that version, but I think that's an issue we can leave for the
hypothetical errparameters() patch.

One problem I noticed is that we don't log parameters when
exec_bind_message fetches the parameter values. So the very first
example problem in testlibpq5 fails to print the values of any
parameters previously seen. I don't think this is a real problem in
practice. You still get the unparseable value in the error message from
the input function, so not having the errdetail() does not seem very
important.

If anybody would like to review it once more, now's the time. I'm
aiming at getting it pushed tomorrow (including the length-limited
appendStringInfoStringQuoted stuff).

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
v16-0001-Add-appendStringInfoStringQuoted.patch text/x-diff 5.4 KB
v16-0002-Emit-parameter-values-during-query-bind-execute-.patch text/x-diff 22.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-12-05 23:17:49 Re: Runtime pruning problem
Previous Message Daniel Gustafsson 2019-12-05 22:45:09 Re: Misleading comment in pg_upgrade.c